New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 591138 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

VideoFrame::WrapVideoFrame() should copy the metadata

Project Member Reported by emir...@chromium.org, Mar 1 2016

Issue description

Currently VideoFrame::WrapVideoFrame() only copies VideoFrameMetadata::END_OF_STREAM value from the wrapped frame's metadata [0]. It doesn't look clear to why it wouldn't copy the whole metadata, unless there are users expecting to start with a clean one. 

[0] https://code.google.com/p/chromium/codesearch#chromium/src/media/base/video_frame.cc&l=497&gs=cpp:media::class-VideoFrame::WrapVideoFrame(const%2520scoped_refptr%253Cmedia::VideoFrame%253E%2520&,%2520const%2520gfx::Rect%2520&,%2520const%2520gfx::Size%2520&)@chromium/../../media/base/video_frame.cc%257Cdef&gsn=WrapVideoFrame&ct=xref_usages
 
Cc: w...@chromium.org
+watk@, I recall: https://codereview.chromium.org/1208693002

Comment 2 by w...@chromium.org, Mar 1 2016

Owner: w...@chromium.org
Status: Assigned (was: Available)
Nice find. I can take this. It looks like the only place  we might be dropping color space metadata is VideoTrackAdapter.

Comment 3 by w...@chromium.org, Mar 1 2016

Owner: tguilbert@chromium.org
Actually, dalecurtis suggested this might be a good ramp-up bug for you, tguilbert@.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24ff05db02b42e939d2f10fdbaae75a83db60120

commit 24ff05db02b42e939d2f10fdbaae75a83db60120
Author: tguilbert <tguilbert@chromium.org>
Date: Fri Mar 04 01:33:27 2016

Modifying VideoFrame to copy all metadata values to wrapped frame in
WrapVideoFrame.

VideoFrame only copied the VideoFrameMetadata::END_OF_STREAM value when creating
a wrapping frame. This change adds a MergeMetadataFrom utility function to
VideoFrameMetadata and uses it to copy all the values from the original frame's
metadata into the wrapped frame.

R=watk@chromium.org
BUG= 591138 
TEST=updated unit tests

Review URL: https://codereview.chromium.org/1752243002

Cr-Commit-Position: refs/heads/master@{#379166}

[modify] https://crrev.com/24ff05db02b42e939d2f10fdbaae75a83db60120/media/base/video_frame.cc
[modify] https://crrev.com/24ff05db02b42e939d2f10fdbaae75a83db60120/media/base/video_frame_metadata.cc
[modify] https://crrev.com/24ff05db02b42e939d2f10fdbaae75a83db60120/media/base/video_frame_metadata.h
[modify] https://crrev.com/24ff05db02b42e939d2f10fdbaae75a83db60120/media/base/video_frame_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment