Media: AudioVideoMetadataParser - Parse rotation metadata.

Media: AudioVideoMetadataParser - Parse rotation metadata.

BUG=332228
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251867

[tommycli, 2014-02-13 00:57:56]

thestig, schekrus: need owner review. thanks!

[tommycli, 2014-02-13 00:58:24]

FYI: 90rotation.mp4 is about 60KB.

[Lei Zhang, 2014-02-13 02:59:25]

https://codereview.chromium.org/148113012/diff/70001/chrome/common/extensions...
File chrome/common/extensions/api/media_galleries.idl (right):

https://codereview.chromium.org/148113012/diff/70001/chrome/common/extensions...
chrome/common/extensions/api/media_galleries.idl:110: // Defined for images and
video. In degrees counterclockwise.
Is this how it's usually done in the video world? seem un-intuitive to do
counterclock-wise

[tommycli, 2014-02-13 22:32:32]

https://codereview.chromium.org/148113012/diff/70001/chrome/common/extensions...
File chrome/common/extensions/api/media_galleries.idl (right):

https://codereview.chromium.org/148113012/diff/70001/chrome/common/extensions...
chrome/common/extensions/api/media_galleries.idl:110: // Defined for images and
video. In degrees counterclockwise.
On 2014/02/13 02:59:26, Lei Zhang wrote:
> Is this how it's usually done in the video world? seem un-intuitive to do
> counterclock-wise

Yeah I agree the comment is confusing. I changed it.

I defined it the same way as exiftool and ffmpeg does it - just looks like
someone rotated the video 90 degrees CCW.

[Lei Zhang, 2014-02-14 02:35:51]

chrome/ lgtm

https://codereview.chromium.org/148113012/diff/170001/media/base/audio_video_...
File media/base/audio_video_metadata_extractor.h (right):

https://codereview.chromium.org/148113012/diff/170001/media/base/audio_video_...
media/base/audio_video_metadata_extractor.h:48: int rotation() const;
move this after track? rotation doesn't seem related to either language or
title.

https://codereview.chromium.org/148113012/diff/170001/media/base/audio_video_...
File media/base/audio_video_metadata_extractor_unittest.cc (right):

https://codereview.chromium.org/148113012/diff/170001/media/base/audio_video_...
media/base/audio_video_metadata_extractor_unittest.cc:30: return
scoped_ptr<AudioVideoMetadataExtractor>();
Isn't this going to case a crash when the caller calls extractor->foo() ?

[tommycli, 2014-02-14 17:06:48]

https://codereview.chromium.org/148113012/diff/170001/media/base/audio_video_...
File media/base/audio_video_metadata_extractor.h (right):

https://codereview.chromium.org/148113012/diff/170001/media/base/audio_video_...
media/base/audio_video_metadata_extractor.h:48: int rotation() const;
On 2014/02/14 02:35:51, Lei Zhang wrote:
> move this after track? rotation doesn't seem related to either language or
> title.

Done. Moved to after 'height' to be consistent with idl file.

https://codereview.chromium.org/148113012/diff/170001/media/base/audio_video_...
File media/base/audio_video_metadata_extractor_unittest.cc (right):

https://codereview.chromium.org/148113012/diff/170001/media/base/audio_video_...
media/base/audio_video_metadata_extractor_unittest.cc:30: return
scoped_ptr<AudioVideoMetadataExtractor>();
On 2014/02/14 02:35:51, Lei Zhang wrote:
> Isn't this going to case a crash when the caller calls extractor->foo() ?

Done.

[tommycli, 2014-02-14 17:07:44]

acolwell: Can I have a media/ review since scherkus is OOO?

[tommycli, 2014-02-18 17:23:53]

On 2014/02/14 17:07:44, tommycli wrote:
> acolwell: Can I have a media/ review since scherkus is OOO?

acolwell: ping

[acolwell GONE FROM CHROMIUM, 2014-02-18 18:12:32]

lgtm

[tommycli, 2014-02-18 18:15:35]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2014-02-18 18:15:40]

On 2014/02/18 18:12:32, acolwell wrote:
> lgtm

thanks!

[commit-bot: I haz the power, 2014-02-18 18:16:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommycli@chromium.org/148113012/290001

[commit-bot: I haz the power, 2014-02-18 18:26:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommycli@chromium.org/148113012/290001

[commit-bot: I haz the power, 2014-02-18 18:34:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommycli@chromium.org/148113012/290001

[commit-bot: I haz the power, 2014-02-18 22:54:01]

Change committed as 251867