Pass a |media_log| to FFmpegVideoDecoder and roll ffmpeg

Pass a |media_log| to FFmpegVideoDecoder.

This allows FFmpegVideoDecoder to log messages for web developers.

As a first attempt, this CL logs a message when FFmpeg fails to
decode a frame that includes the message from FFmpeg and details
about the failed buffer.

Includes FFmpeg DEPS roll to expose these messages.

    Roll src/third_party/ffmpeg/ 6d2e44445..9751ef06f (1 commit)
    
    https://chromium.googlesource.com/chromium/third_party/ffmpeg.git/+log/6d2e444456b7..9751ef06f90f
    
    $ git log 6d2e44445..9751ef06f --date=short --no-merges --format='%ad %ae %s'
    2017-03-23 sandersd Expose av_strerror() via ffmpeg.sigs.


BUG=TBD
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2768713002
Cr-Commit-Position: refs/heads/master@{#459548}
Committed: https://chromium.googlesource.com/chromium/src/+/68e83a343963981524faa8e03fc6cc274ecc8409

[sandersd (OOO until July 31), 2017-03-22 00:10:55]

Description was changed from

==========
Pass a |media_log| to FFmpegVideoDecoder.

This allows FFmpegVideoDecoder to log messages for web developers.

As a first attempt, this CL logs a message when FFmpeg fails to
decode a frame that includes the message from FFmpeg and details
about the failed buffer.

BUG=TBD
==========

to

==========
Pass a |media_log| to FFmpegVideoDecoder.

This allows FFmpegVideoDecoder to log messages for web developers.

As a first attempt, this CL logs a message when FFmpeg fails to
decode a frame that includes the message from FFmpeg and details
about the failed buffer.

BUG=TBD
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[sandersd (OOO until July 31), 2017-03-22 00:12:36]

sandersd@chromium.org changed reviewers:
+ dalecurtis@chromium.org, wolenetz@chromium.org

[sandersd (OOO until July 31), 2017-03-22 00:12:36]

sandersd@chromium.org changed required reviewers:
+ wolenetz@chromium.org

[sandersd (OOO until July 31), 2017-03-22 00:12:37]

We probably want to at least trim down the information about the failing buffer,
but it's a start.

If this is an agreeable approach then the same is easy to do for FFmpegDemuxer
and VpxVideoDecoder.

[DaleCurtis, 2017-03-22 00:43:16]

Seems okay.

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
File media/filters/ffmpeg_video_decoder.cc (right):

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
media/filters/ffmpeg_video_decoder.cc:38: av_make_error_string(errbuf,
AV_ERROR_MAX_STRING_SIZE, errnum);
Will need an ffmpeg roll to use this in component windows builds; need to update
sigs file.

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
File media/filters/ffmpeg_video_decoder.h (right):

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
media/filters/ffmpeg_video_decoder.h:32:
FFmpegVideoDecoder(scoped_refptr<MediaLog> media_log);
explicit

[wolenetz, 2017-03-22 19:31:50]

Looks pretty good (+1 to Dale's comments).
Do the unit tests pass? I suspect you might need to add some extra expectations
for unit tests that use a StrictMock MockMediaLog in test cases where ffmpeg
decoder is expected to fail.

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
File media/filters/ffmpeg_video_decoder.cc (right):

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
media/filters/ffmpeg_video_decoder.cc:36: std::string AVErrorString(int errnum)
{
If the plan is to put similar into other ffmpeg use cases like FFmpegDemuxer,
please consider moving this to ffmpeg_common.cc

[sandersd (OOO until July 31), 2017-03-23 23:34:26]

The CQ bit was checked by sandersd@chromium.org to run a CQ dry run

[sandersd (OOO until July 31), 2017-03-23 23:34:36]

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
File media/filters/ffmpeg_video_decoder.cc (right):

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
media/filters/ffmpeg_video_decoder.cc:36: std::string AVErrorString(int errnum)
{
On 2017/03/22 19:31:50, wolenetz wrote:
> If the plan is to put similar into other ffmpeg use cases like FFmpegDemuxer,
> please consider moving this to ffmpeg_common.cc

Done.

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
media/filters/ffmpeg_video_decoder.cc:38: av_make_error_string(errbuf,
AV_ERROR_MAX_STRING_SIZE, errnum);
On 2017/03/22 00:43:16, DaleCurtis wrote:
> Will need an ffmpeg roll to use this in component windows builds; need to
update
> sigs file.

Done.

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
File media/filters/ffmpeg_video_decoder.h (right):

https://codereview.chromium.org/2768713002/diff/1/media/filters/ffmpeg_video_...
media/filters/ffmpeg_video_decoder.h:32:
FFmpegVideoDecoder(scoped_refptr<MediaLog> media_log);
On 2017/03/22 00:43:16, DaleCurtis wrote:
> explicit

Done.

[commit-bot: I haz the power, 2017-03-23 23:34:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[DaleCurtis, 2017-03-23 23:36:17]

Followup cls for FFmpeg audio decoder and VpxVideoDecoder?

[DaleCurtis, 2017-03-23 23:36:22]

lgtm

[sandersd (OOO until July 31), 2017-03-23 23:38:31]

On 2017/03/23 23:36:17, DaleCurtis wrote:
> Followup cls for FFmpeg audio decoder and VpxVideoDecoder?

That's the plan.

[commit-bot: I haz the power, 2017-03-23 23:38:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-23 23:38:46]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)
  win_optional_gpu_tests_rel on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)

[sandersd (OOO until July 31), 2017-03-23 23:39:12]

On 2017/03/23 23:38:31, sandersd wrote:
> On 2017/03/23 23:36:17, DaleCurtis wrote:
> > Followup cls for FFmpeg audio decoder and VpxVideoDecoder?
> 
> That's the plan.

(Sadly VDAs will have to wait for the mojoification of MediaLog.)

[sandersd (OOO until July 31), 2017-03-23 23:42:39]

The CQ bit was checked by sandersd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-23 23:43:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sandersd (OOO until July 31), 2017-03-23 23:43:21]

sandersd@chromium.org changed reviewers:
+ bbudge@chromium.org

[sandersd (OOO until July 31), 2017-03-23 23:43:21]

bbudge@chromium.org: Please review changes in
content/renderer/pepper/video_decoder_shim.cc.

[commit-bot: I haz the power, 2017-03-23 23:48:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-23 23:48:09]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[sandersd (OOO until July 31), 2017-03-23 23:50:03]

On 2017/03/23 23:43:21, sandersd wrote:
> mailto:bbudge@chromium.org: Please review changes in
> content/renderer/pepper/video_decoder_shim.cc.

Looks like rolling to the review commit does not work on the bots unless that
commit is pushed to a branch.

I'll just make the commit and then can test this CL with a real commit hash.

[sandersd (OOO until July 31), 2017-03-23 23:52:02]

The CQ bit was checked by sandersd@chromium.org to run a CQ dry run

[wolenetz, 2017-03-23 23:54:19]

lgtm %

https://codereview.chromium.org/2768713002/diff/60001/DEPS
File DEPS (right):

https://codereview.chromium.org/2768713002/diff/60001/DEPS#newcode208
DEPS:208: Var('chromium_git') + '/chromium/third_party/ffmpeg.git' + '@' +
'09fe964c10d868b829ee7ede18040c801c0780f2',
This looks wrong versus
https://chromium.googlesource.com/chromium/third_party/ffmpeg/
This was probably your local branch's hash, not the hash of what landed in the
repo.

[wolenetz, 2017-03-23 23:56:17]

On 2017/03/23 23:54:19, wolenetz wrote:
> lgtm %
> 
> https://codereview.chromium.org/2768713002/diff/60001/DEPS
> File DEPS (right):
> 
> https://codereview.chromium.org/2768713002/diff/60001/DEPS#newcode208
> DEPS:208: Var('chromium_git') + '/chromium/third_party/ffmpeg.git' + '@' +
> '09fe964c10d868b829ee7ede18040c801c0780f2',
> This looks wrong versus
> https://chromium.googlesource.com/chromium/third_party/ffmpeg/
> This was probably your local branch's hash, not the hash of what landed in the
> repo.

also % my earlier question and offline chat about StrictMock MockMediaLog
coverage of at least one AVErrorToString error message.

[sandersd (OOO until July 31), 2017-03-24 00:38:09]

> also % my earlier question and offline chat about StrictMock MockMediaLog
> coverage of at least one AVErrorToString error message.

This was one of the easiest unit tests I've written for Chrome.

I did base it on an explicit substring though, so it's somewhat brittle.

[sandersd (OOO until July 31), 2017-03-24 00:40:17]

The CQ bit was checked by sandersd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-24 00:40:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sandersd (OOO until July 31), 2017-03-24 00:43:51]

Description was changed from

==========
Pass a |media_log| to FFmpegVideoDecoder.

This allows FFmpegVideoDecoder to log messages for web developers.

As a first attempt, this CL logs a message when FFmpeg fails to
decode a frame that includes the message from FFmpeg and details
about the failed buffer.

BUG=TBD
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Pass a |media_log| to FFmpegVideoDecoder.

This allows FFmpegVideoDecoder to log messages for web developers.

As a first attempt, this CL logs a message when FFmpeg fails to
decode a frame that includes the message from FFmpeg and details
about the failed buffer.

Includes FFmpeg DEPS roll to expose these messages.

    Roll src/third_party/ffmpeg/ 6d2e44445..9751ef06f (1 commit)
    
   
https://chromium.googlesource.com/chromium/third_party/ffmpeg.git/+log/6d2e44...
    
    $ git log 6d2e44445..9751ef06f --date=short --no-merges --format='%ad %ae
%s'
    2017-03-23 sandersd Expose av_strerror() via ffmpeg.sigs.


BUG=TBD
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2017-03-24 00:51:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-24 00:51:03]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[wolenetz, 2017-03-24 17:38:41]

On 2017/03/24 00:38:09, sandersd wrote:
> > also % my earlier question and offline chat about StrictMock MockMediaLog
> > coverage of at least one AVErrorToString error message.
> 
> This was one of the easiest unit tests I've written for Chrome.
> 
> I did base it on an explicit substring though, so it's somewhat brittle.

Brittleness is an unfortunate side-effect of such verification using StrictMock,
but it does give us confidence that the logging path itself is basically
working. Thanks for adding it.

lgtm2 (% fixing the compile/build issues the bots show)

[sandersd (OOO until July 31), 2017-03-24 18:04:33]

The CQ bit was checked by sandersd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-24 18:05:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-24 18:23:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-24 18:23:15]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[bbudge, 2017-03-24 18:40:17]

content/renderer/pepper lgtm

[sandersd (OOO until July 31), 2017-03-24 18:53:03]

The CQ bit was checked by sandersd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-24 18:53:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-24 20:37:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-24 20:37:53]

Dry run: This issue passed the CQ dry run.

[sandersd (OOO until July 31), 2017-03-24 20:38:51]

The CQ bit was checked by sandersd@chromium.org

[sandersd (OOO until July 31), 2017-03-24 20:38:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, wolenetz@chromium.org, bbudge@chromium.org
Link to the patchset: https://codereview.chromium.org/2768713002/#ps130001
(title: "Update cast sender unittest.")

[commit-bot: I haz the power, 2017-03-24 20:39:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-24 20:59:03]

CQ is committing da patch.

Bot data: {"patchset_id": 130001, "attempt_start_ts": 1490387931347190,
"parent_rev": "a76f0b59ca704c56c693b2390a8033f834b08a3f", "commit_rev":
"68e83a343963981524faa8e03fc6cc274ecc8409"}

[commit-bot: I haz the power, 2017-03-24 20:59:46]

Description was changed from

==========
Pass a |media_log| to FFmpegVideoDecoder.

This allows FFmpegVideoDecoder to log messages for web developers.

As a first attempt, this CL logs a message when FFmpeg fails to
decode a frame that includes the message from FFmpeg and details
about the failed buffer.

Includes FFmpeg DEPS roll to expose these messages.

    Roll src/third_party/ffmpeg/ 6d2e44445..9751ef06f (1 commit)
    
   
https://chromium.googlesource.com/chromium/third_party/ffmpeg.git/+log/6d2e44...
    
    $ git log 6d2e44445..9751ef06f --date=short --no-merges --format='%ad %ae
%s'
    2017-03-23 sandersd Expose av_strerror() via ffmpeg.sigs.


BUG=TBD
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Pass a |media_log| to FFmpegVideoDecoder.

This allows FFmpegVideoDecoder to log messages for web developers.

As a first attempt, this CL logs a message when FFmpeg fails to
decode a frame that includes the message from FFmpeg and details
about the failed buffer.

Includes FFmpeg DEPS roll to expose these messages.

    Roll src/third_party/ffmpeg/ 6d2e44445..9751ef06f (1 commit)
    
   
https://chromium.googlesource.com/chromium/third_party/ffmpeg.git/+log/6d2e44...
    
    $ git log 6d2e44445..9751ef06f --date=short --no-merges --format='%ad %ae
%s'
    2017-03-23 sandersd Expose av_strerror() via ffmpeg.sigs.


BUG=TBD
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2768713002
Cr-Commit-Position: refs/heads/master@{#459548}
Committed:
https://chromium.googlesource.com/chromium/src/+/68e83a343963981524faa8e03fc6...
==========

[commit-bot: I haz the power, 2017-03-24 20:59:48]

Committed patchset #8 (id:130001) as
https://chromium.googlesource.com/chromium/src/+/68e83a343963981524faa8e03fc6...