Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2365103002:
Send the h264 SPS and PPS configuration parameters to AVDA (Closed)
|
Created:
4 years, 3 months ago by watk Modified:
4 years, 2 months ago CC:
liberato (no reviews please), chromium-reviews, posciak+watch_chromium.org, feature-media-reviews_chromium.org, avayvod+watch_chromium.org, mlamouri+watch-media_chromium.org, piman+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionSend the h264 SPS and PPS configuration parameters to AVDA
Previously we relied on MediaCodec's ability to get these from the
bitstream. But in at least one case (see bug) MediaCodec works more
reliably when we pass these when initializing it.
BUG=649185
Committed: https://crrev.com/18e1a10a685a93bbcd528da9d61387341fb8484b
Cr-Commit-Position: refs/heads/master@{#426091}
Patch Set 1 #Patch Set 2 : Move parsing #Patch Set 3 : Fix unittests #Patch Set 4 : Fix array init syntax #
Total comments: 6
Patch Set 5 : Make it a non-member function #
Total comments: 3
Patch Set 6 : actually fix array init syntax #Patch Set 7 : constexpr #Patch Set 8 : rename extra_data to avcc #Patch Set 9 : Moved parsing to the renderer #
Total comments: 3
Patch Set 10 : formatting #Patch Set 11 : ifdef ExtractSpsAndPps for Android so it's not an unused function #Patch Set 12 : Condition the SPS/PPS passing on PROPRIETARY_CODECS too #Patch Set 13 : Shot in the dark #Messages
Total messages: 78 (42 generated)
The CQ bit was checked by watk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
watk@chromium.org changed reviewers: + sandersd@chromium.org
PTAL sandersd FYI liberato
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
posciak@chromium.org changed reviewers: + posciak@chromium.org
Could we perhaps use base::H264Parser in AVDA itself (like VAVDA and other VDAs do) to extract these from the stream, to avoid requiring clients to parse?
On 2016/09/26 11:07:13, Pawel Osciak wrote: > Could we perhaps use base::H264Parser in AVDA itself (like VAVDA and other VDAs > do) to extract these from the stream, to avoid requiring clients to parse? Yep will do. I was trying to keep parsing in the renderer for security reasons, but in an offline chat sandersd said he thinks it's not worth it. Plus VDAv2 will default to passing extra_data() instead of sps pps, so the interfaces will be more similar if I do that now.
The CQ bit was checked by watk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #3 (id:40001) has been deleted
The CQ bit was checked by watk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
On 2016/09/26 20:22:13, watk wrote: > PTAL ping
lgtm, just nits. https://codereview.chromium.org/2365103002/diff/80001/media/base/android/vide... File media/base/android/video_decoder_job.cc (right): https://codereview.chromium.org/2365103002/diff/80001/media/base/android/vide... media/base/android/video_decoder_job.cc:140: const std::vector<uint8_t> csd; empty_csd? https://codereview.chromium.org/2365103002/diff/80001/media/gpu/android_video... File media/gpu/android_video_decode_accelerator.cc (right): https://codereview.chromium.org/2365103002/diff/80001/media/gpu/android_video... media/gpu/android_video_decode_accelerator.cc:1612: if (codec_config_->codec_ != kCodecH264 || extra_data.empty()) I don't like this dependency, it seems to me that this should be passed in. (And I might rename the method to ExtractCsd, or even Initialize_ExtractCsd to make its purpose more clear.) https://codereview.chromium.org/2365103002/diff/80001/media/gpu/android_video... media/gpu/android_video_decode_accelerator.cc:1618: return; Perhaps we should return a bool? Explicitly ignoring the result is reasonable (with a comment), but this data should always be correct.
The CQ bit was checked by watk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
watk@chromium.org changed reviewers: + dcheng@chromium.org
Thanks, dcheng@ PTAL at media_param_traits_macros.h https://codereview.chromium.org/2365103002/diff/80001/media/base/android/vide... File media/base/android/video_decoder_job.cc (right): https://codereview.chromium.org/2365103002/diff/80001/media/base/android/vide... media/base/android/video_decoder_job.cc:140: const std::vector<uint8_t> csd; On 2016/09/27 20:03:16, sandersd wrote: > empty_csd? Done. https://codereview.chromium.org/2365103002/diff/80001/media/gpu/android_video... File media/gpu/android_video_decode_accelerator.cc (right): https://codereview.chromium.org/2365103002/diff/80001/media/gpu/android_video... media/gpu/android_video_decode_accelerator.cc:1612: if (codec_config_->codec_ != kCodecH264 || extra_data.empty()) On 2016/09/27 20:03:16, sandersd wrote: > I don't like this dependency, it seems to me that this should be passed in. (And > I might rename the method to ExtractCsd, or even Initialize_ExtractCsd to make > its purpose more clear.) Done. https://codereview.chromium.org/2365103002/diff/80001/media/gpu/android_video... media/gpu/android_video_decode_accelerator.cc:1618: return; On 2016/09/27 20:03:16, sandersd wrote: > Perhaps we should return a bool? Explicitly ignoring the result is reasonable > (with a comment), but this data should always be correct. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
https://codereview.chromium.org/2365103002/diff/100001/media/gpu/android_vide... File media/gpu/android_video_decode_accelerator.cc (right): https://codereview.chromium.org/2365103002/diff/100001/media/gpu/android_vide... media/gpu/android_video_decode_accelerator.cc:144: const std::array<uint8_t, 4> prefix = {0, 0, 0, 1}; constexpr https://codereview.chromium.org/2365103002/diff/100001/media/gpu/ipc/common/m... File media/gpu/ipc/common/media_param_traits_macros.h (right): https://codereview.chromium.org/2365103002/diff/100001/media/gpu/ipc/common/m... media/gpu/ipc/common/media_param_traits_macros.h:35: IPC_STRUCT_TRAITS_MEMBER(extra_data) Just to make this review easier... this is data getting sent from the browser to the gpu process, right? Not from the renderer to gpu/browser or gpu to browser?
On 2016/09/27 22:41:15, dcheng wrote: > https://codereview.chromium.org/2365103002/diff/100001/media/gpu/android_vide... > File media/gpu/android_video_decode_accelerator.cc (right): > > https://codereview.chromium.org/2365103002/diff/100001/media/gpu/android_vide... > media/gpu/android_video_decode_accelerator.cc:144: const std::array<uint8_t, 4> > prefix = {0, 0, 0, 1}; > constexpr > > https://codereview.chromium.org/2365103002/diff/100001/media/gpu/ipc/common/m... > File media/gpu/ipc/common/media_param_traits_macros.h (right): > > https://codereview.chromium.org/2365103002/diff/100001/media/gpu/ipc/common/m... > media/gpu/ipc/common/media_param_traits_macros.h:35: > IPC_STRUCT_TRAITS_MEMBER(extra_data) > Just to make this review easier... this is data getting sent from the browser to > the gpu process, right? Not from the renderer to gpu/browser or gpu to browser? Sorry, it's going renderer -> gpu. The gpu process hosts the video decode accelerator, and the renderer sends it these initialization parameters, and buffers to decode.
The CQ bit was checked by watk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
To expand upon that ^. We are now parsing extra_data in AVDA which is running in the gpu process. But this isn't unusual for VDAs; at least the Mac and ChromeOS ones do a lot of h264 bitstream parsing. https://codereview.chromium.org/2365103002/diff/100001/media/gpu/android_vide... File media/gpu/android_video_decode_accelerator.cc (right): https://codereview.chromium.org/2365103002/diff/100001/media/gpu/android_vide... media/gpu/android_video_decode_accelerator.cc:144: const std::array<uint8_t, 4> prefix = {0, 0, 0, 1}; On 2016/09/27 22:41:15, dcheng wrote: > constexpr Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Friendly ping
On 2016/09/27 22:56:03, watk wrote: > To expand upon that ^. We are now parsing extra_data in AVDA which is running in > the gpu process. But this isn't unusual for VDAs; at least the Mac and ChromeOS > ones do a lot of h264 bitstream parsing. I assume the VDA on Mac / CrOS also run in the GPU process. However, I tried to find a place in media/gpu that uses the parsing bits from box_definitions.h and I wasn't able to. Can you point me at where this is happening today> > > https://codereview.chromium.org/2365103002/diff/100001/media/gpu/android_vide... > File media/gpu/android_video_decode_accelerator.cc (right): > > https://codereview.chromium.org/2365103002/diff/100001/media/gpu/android_vide... > media/gpu/android_video_decode_accelerator.cc:144: const std::array<uint8_t, 4> > prefix = {0, 0, 0, 1}; > On 2016/09/27 22:41:15, dcheng wrote: > > constexpr > > Done.
On 2016/09/30 06:48:07, dcheng wrote: > On 2016/09/27 22:56:03, watk wrote: > > To expand upon that ^. We are now parsing extra_data in AVDA which is running > in > > the gpu process. But this isn't unusual for VDAs; at least the Mac and > ChromeOS > > ones do a lot of h264 bitstream parsing. > > I assume the VDA on Mac / CrOS also run in the GPU process. Yes. > However, I tried to > find a place in media/gpu that uses the parsing bits from box_definitions.h and > I wasn't able to. Can you point me at where this is happening today> We use media::H264Parser to parse, e.g. here: https://cs.chromium.org/chromium/src/media/gpu/h264_decoder.cc?rcl=0&l=1306 (here we get all the contents of NALUs), or here: https://cs.chromium.org/chromium/src/media/gpu/v4l2_video_decode_accelerator.... (here we only need to find the NALU boundaries). But we get them from the raw stream passed to Decode(), we don't need to pass any extra data. I'm not sure why we need to pass extra data separately here. Do we need to pass it? Could we parse and use SPS and PPS data that is already present in the BitstreamBuffer passed to Decode()...?
> I'm not sure why we need to pass extra data separately here. Do we need to
pass
> it? Could we parse and use SPS and PPS data that is already present in the
> BitstreamBuffer passed to Decode()...?
That's possible, but has a significant drawback:
- AVDA would need to start using H264Parser, and understand all the different
edge cases there. |extra_data| is both safer to parse and easier to interpret.
|extra_data| also opens up future improvements:
- This method allows us to test for support of the stream in Initialize() and
return a correct status. Note that on Android, the H.264 profile information is
not accurate.
- VTVDA would like to do the same, specifically so that it can reject streams
that VideoToolbox would internally use a software decoder for.
(Once Decode() is called, we can't fall back to a different decoder.)
And the main reason I convinced watk@ to do it this way:
- Consistency with the rest of the media pipeline. (i.e. this is how VDAv2
works, and so AVDA is going there eventually.)
On 2016/09/30 17:32:15, sandersd wrote:
> > I'm not sure why we need to pass extra data separately here. Do we need to
> pass
> > it? Could we parse and use SPS and PPS data that is already present in the
> > BitstreamBuffer passed to Decode()...?
>
> That's possible, but has a significant drawback:
> - AVDA would need to start using H264Parser, and understand all the
different
> edge cases there. |extra_data| is both safer to parse and easier to interpret.
H264Parser should encapsulate all the logic, so that users should hopefully not
have to worry about edge cases and stream specifics, and only say something
like:
while (1) {
H264NALU nalu;
res = parser_.AdvanceToNextNALU(&nalu);
if (res == H264Parser::kEOStream)
break;
else if (res != H264Parser::kOk)
Error();
switch (nalu->nal_unit_type) {
case H264NALU::kSPS:
PushNALUOntoCsd0List(nalu);
break;
case H264NALU::kPPS:
PushNALUOntoCsd1List(nalu);
break;
default:
break;
}
}
On the other hand, in my opinion, a custom, raw-data field "extra_data" that is
not clearly defined for all cases and all platforms, apart from a specific
H264+AVDA case, could also be unsafe and difficult to parse/handle for both VDAs
and their clients.
>
> |extra_data| also opens up future improvements:
> - This method allows us to test for support of the stream in Initialize()
and
> return a correct status. Note that on Android, the H.264 profile information
is
> not accurate.
extra_data field may in the current form be too ambiguous in my opinion. For a
platform/codec independent interface like VDA, it would be great if we could
avoid "Optional codec configuration"-type fields, as it may be difficult to use
for clients and problematic for VDA impls as well to handle.
If we strongly prefer not to use H264Parser, could we consider adding the
required information (SPS and PPS structs) to the Config directly instead?
In the current form, this is AVDA-specific, H264-specific field, and it's not
clear from the definition if it's required even for H264 with AVDA. So I would
really prefer to avoid this kind of addition to the VDA interface.
Also, although I'm likely not correct here, since I'd expect Chrome IPC
mechanisms to have safeguards against such situations, I wonder if having a
custom-sized vector of bytes passed across IPC is recommended, as a malicious
client could push a big amount of data and bottleneck the IPC to GPU process
this way?
> - VTVDA would like to do the same, specifically so that it can reject
streams
> that VideoToolbox would internally use a software decoder for.
> (Once Decode() is called, we can't fall back to a different decoder.)
>
If so, this may not help in all cases, as you may face the same issue if the
SPS/PPS changes mid-stream, which is a normal situation for H264. Even if we
pass initial SPS and PPS directly as extra_data, we may will still have the same
issue later on mid-stream, and won't be able to fall back to a different decoder
then?
> And the main reason I convinced watk@ to do it this way:
> - Consistency with the rest of the media pipeline. (i.e. this is how VDAv2
> works, and so AVDA is going there eventually.)
Is VDAv2 API interface (class definition) available somewhere I could review it?
What do we mean exactly by "this way" vs. using the existing BitstreamBuffer
contents? Is the plan to pass container data as well, not only the raw stream
extracted from the container?
I'll provide short replies inline, but it sounds like we should schedule a discussion to resolve this debate. [Skipping the non-technical parts since we disagree so entirely about H264Parser vs AVCDecoderConfigurationRecord.] > If so, this may not help in all cases, as you may face the same issue if the > SPS/PPS changes mid-stream, which is a normal situation for H264. Even if we > pass initial SPS and PPS directly as extra_data, we may will still have the same > issue later on mid-stream, and won't be able to fall back to a different decoder > then? We get a chance to fall back safely each time there is an explicit configuration change. Right now the VDA interface does not expose this, it is instead hidden inside the Annex B converter. For the bug this CL in particular solves, we don't need to make more API changes immediately. Android seems to handle the config change detection itself correctly once we bootstrap it. > > And the main reason I convinced watk@ to do it this way: > > - Consistency with the rest of the media pipeline. (i.e. this is how VDAv2 > > works, and so AVDA is going there eventually.) > > Is VDAv2 API interface (class definition) available somewhere I could review it? > What do we mean exactly by "this way" vs. using the existing BitstreamBuffer > contents? Is the plan to pass container data as well, not only the raw stream > extracted from the container? The VDAv2 API is media::VideoDecoder. media::VideoDecoder::Initialize() includes |extra_data|, so VDAv2 does as well. There is no change to bitstream buffer contents or interpretation. (Although note that under VDAv2, VTVDA will be opting out of Annex B conversion for improved performance and reliability, so |extra_data| is the only place the SPS and PPS will appear.)
Dcheng@, should we be worried about letting the renderer send a vector to the gpu process? Are there safeguards on the size of vectors sent over IPC?
On 2016/10/04 18:50:26, watk wrote: > Dcheng@, should we be worried about letting the renderer send a vector to the > gpu process? Are there safeguards on the size of vectors sent over IPC? What sorts of issues would you be worried about? - IPC will validate that the actual length of the data sent across and the declared length of the vector match - IPC will validate the contents of the vector (though currently this is just a byte buffer, so there really is no extra validation) But if there's code parsing things out of a byte buffer manually, then that needs to also be carefully validated to make sure there's no possible out-of-bounds reads, etc.
On 2016/10/04 19:18:57, dcheng wrote: > On 2016/10/04 18:50:26, watk wrote: > > Dcheng@, should we be worried about letting the renderer send a vector to the > > gpu process? Are there safeguards on the size of vectors sent over IPC? > > What sorts of issues would you be worried about? The concern that was raised was over the ability to DoS the GPU process by sending extremely large messages. We're hoping to find out that the IPC system protects itself against such things at a low level, since there is not a huge amount we can do ourselves. If vectors in particular are more dangerous we can switch to passing this information via SHM.
On 2016/10/04 19:37:49, sandersd wrote: > On 2016/10/04 19:18:57, dcheng wrote: > > On 2016/10/04 18:50:26, watk wrote: > > > Dcheng@, should we be worried about letting the renderer send a vector to > the > > > gpu process? Are there safeguards on the size of vectors sent over IPC? > > > > What sorts of issues would you be worried about? > > The concern that was raised was over the ability to DoS the GPU process by > sending extremely large messages. > > We're hoping to find out that the IPC system protects itself against such things > at a low level, since there is not a huge amount we can do ourselves. If vectors > in particular are more dangerous we can switch to passing this information via > SHM. Friendly ping dcheng@. I took a quick look and don't see anything limiting the size of IPC messages.
On 2016/10/07 20:49:36, watk wrote: > On 2016/10/04 19:37:49, sandersd wrote: > > On 2016/10/04 19:18:57, dcheng wrote: > > > On 2016/10/04 18:50:26, watk wrote: > > > > Dcheng@, should we be worried about letting the renderer send a vector to > > the > > > > gpu process? Are there safeguards on the size of vectors sent over IPC? > > > > > > What sorts of issues would you be worried about? > > > > The concern that was raised was over the ability to DoS the GPU process by > > sending extremely large messages. > > > > We're hoping to find out that the IPC system protects itself against such > things > > at a low level, since there is not a huge amount we can do ourselves. If > vectors > > in particular are more dangerous we can switch to passing this information via > > SHM. > > Friendly ping dcheng@. I took a quick look and don't see anything limiting the > size of IPC messages. Oops, sorry. There isn't, but that's not really a problem unique to this IPC: it's true of the entire IPC system in general. Even if we limit the size of one individual IPC, a renderer / GPU can easily flood the message queue instead.
The CQ bit was checked by watk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Ok, extra_data renamed to avcc in the VDA interface. PTAL posciak@
Forgot to say: thanks for the info dcheng@.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Talked offline with dcheng@. He'd prefer we didn't run new parsing in the gpu process. I'm fine with changing it back to how it was originally: parsing in the renderer and passing the sps/pps in the VDA config. If anyone objects to this please speak up before I do it. I added a fuzzer for this also because it was trivial https://codereview.chromium.org/2401963003
Moved the parsing back to the renderer.
https://codereview.chromium.org/2365103002/diff/180001/media/gpu/android_vide... File media/gpu/android_video_decode_accelerator.cc (right): https://codereview.chromium.org/2365103002/diff/180001/media/gpu/android_vide... media/gpu/android_video_decode_accelerator.cc:389: codec_config_->csd1_ = config.pps; Is there anything interesting for the GPU to validate here? For example, what happens if sps or pps don't start with the prefix?
On 2016/10/12 05:57:13, dcheng wrote: > https://codereview.chromium.org/2365103002/diff/180001/media/gpu/android_vide... > File media/gpu/android_video_decode_accelerator.cc (right): > > https://codereview.chromium.org/2365103002/diff/180001/media/gpu/android_vide... > media/gpu/android_video_decode_accelerator.cc:389: codec_config_->csd1_ = > config.pps; > Is there anything interesting for the GPU to validate here? For example, what > happens if sps or pps don't start with the prefix? We just pass it along to Android's MediaCodec, which will just fail to initialize if it's malformed. Validating the format is equivalent to parsing them AFAICT, so it doesn't seem worth doing to me.
LGTM https://codereview.chromium.org/2365103002/diff/180001/media/base/android/sdk... File media/base/android/sdk_media_codec_bridge.h (right): https://codereview.chromium.org/2365103002/diff/180001/media/base/android/sdk... media/base/android/sdk_media_codec_bridge.h:145: csd0, // Codec specific data. See MediaCodec docs. FWIW, the wrapping here might be a bit nicer if the comment is on the previous line
Thanks! https://codereview.chromium.org/2365103002/diff/180001/media/base/android/sdk... File media/base/android/sdk_media_codec_bridge.h (right): https://codereview.chromium.org/2365103002/diff/180001/media/base/android/sdk... media/base/android/sdk_media_codec_bridge.h:145: csd0, // Codec specific data. See MediaCodec docs. On 2016/10/17 22:39:49, dcheng wrote: > FWIW, the wrapping here might be a bit nicer if the comment is on the previous > line Done.
The CQ bit was checked by watk@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sandersd@chromium.org, dcheng@chromium.org Link to the patchset: https://codereview.chromium.org/2365103002/#ps200001 (title: "formatting")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: 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-...) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
The CQ bit was checked by watk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
The CQ bit was checked by watk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...) win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by watk@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org, sandersd@chromium.org Link to the patchset: https://chromiumcodereview.appspot.com/2365103002/#ps260001 (title: "Shot in the dark")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Exceeded global retry quota
The CQ bit was checked by watk@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #13 (id:260001)
Message was sent while issue was closed.
Description was changed from ========== Send the h264 SPS and PPS configuration parameters to AVDA Previously we relied on MediaCodec's ability to get these from the bitstream. But in at least one case (see bug) MediaCodec works more reliably when we pass these when initializing it. BUG=649185 ========== to ========== Send the h264 SPS and PPS configuration parameters to AVDA Previously we relied on MediaCodec's ability to get these from the bitstream. But in at least one case (see bug) MediaCodec works more reliably when we pass these when initializing it. BUG=649185 Committed: https://crrev.com/18e1a10a685a93bbcd528da9d61387341fb8484b Cr-Commit-Position: refs/heads/master@{#426091} ==========
Message was sent while issue was closed.
Patchset 13 (id:??) landed as https://crrev.com/18e1a10a685a93bbcd528da9d61387341fb8484b Cr-Commit-Position: refs/heads/master@{#426091} |
