Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1879353003:
Attempt decoder fallback if first decode fails (Closed)
DescriptionAttempt decoder fallback if first decode fails
This change is a manual merge of DaleCurtis' CL, along with unittests
and and coverage for edge cases, such as handling parallel decode
requests, config changes and EOS.
Updated original description:
There are several instances where a GPU video decoder will fail on
the first decode even though it reported successfull initialization.
In these cases try to fallback to a software decoder if possible.
This works by queuing the first buffers sent to Decode() to a pending
buffer queue, before any output callbacks have occurred. If decode
fails before output is it will attempt to reselect a decoder. If this
is successful the pending decode buffers will be copied to a fallback
buffer queue, which will be fed in at an appropriate rate to the new
decoder.
Pending decode callbacks from the previous decoder are invalidated by
way of a second weak_ptr factory for the decode callback.
BUG=128837
TEST=Ran media_unittests successfully, manual (works as expected for MSE, bug 605790 for SRC=)
Committed: https://crrev.com/e384d5d6be1ee2c196df5dc94c50419faf82f2a2
Cr-Commit-Position: refs/heads/master@{#388990}
Patch Set 1 #
Total comments: 12
Patch Set 2 : Addressing comments, improving logic #Patch Set 3 : Comments #
Total comments: 11
Patch Set 4 : #Patch Set 5 : #Patch Set 6 : #
Total comments: 16
Patch Set 7 : #Patch Set 8 : Addressing offline comments #Patch Set 9 : #
Total comments: 2
Patch Set 10 : Addressing comment #Patch Set 11 : new test for EOS flushing edge case #
Messages
Total messages: 28 (9 generated)
tguilbert@chromium.org changed reviewers: + dalecurtis@chromium.org, sandersd@chromium.org
What is the proper way to link to Dale's original CL? https://codereview.chromium.org/1720343002 Is there a bug number for this? There is one TODO that is actually a question for reviewers. Thank you!
https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:325: pending_buffers_.push_back(buffer); Why don't we queue EOS buffers? https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:330: weak_factory_.GetWeakPtr(), sequence_token_, If OnDecodeDone() should never be called when the sequence token is old, it may be simpler to create a second WeakPtrFactory just for this Bind(). Then you can call Invalidate() to prevent any more callbacks from coming. (You can still vend new weak pointers from the same factory.) https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:483: if (!pending_buffers_.empty() && previous_decoder_) { It seems that there really needs to be two lists of pending buffers; one is what has been fed in so far, the other is what has yet to be sent to the current decoder. Then when switching decoders the second list can just be reset to a copy of the first, and there are no strange conditions to check. It may be easiest to do that by splitting Decode() into two methods: a wrapper that records to that list (Decode()), and an internal method that actually dispatches (DoDecode() or DecodeInternal()). Then you can call the internal one here and don't need to worry about the pending list being walked on.
Nice tests! https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:441: if (!pending_buffers_.empty() && Multiline if needs {} https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:514: // Save valid buffers to be consumed by the new decoder You'll need to set STATE_ERROR in the non-kOk case and adapt OnDecoderReinitialized to handle it I think. https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:536: // Pending buffers might not match the reinitiliazed decoder's new config DCHECK(pending_buffers_.empty()); ? I think this can't happen with your above code. Also misspelled reinitialized.
Description was changed from ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() before any output callbacks have occurred. If decode fails before output is received it will attempt to reselect a decoder. If this is successful the pending decoders will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a sequence token. BUG=??? TEST=Ran media_unittests successfully ========== to ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=??? TEST=Ran media_unittests successfully ==========
https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:325: pending_buffers_.push_back(buffer); On 2016/04/14 00:38:04, sandersd wrote: > Why don't we queue EOS buffers? In order to reinitialize the decoders on a config change, we flush the decoder by sending it an EOS. The first buffer that would be queued in that case is an EOS. I will make sure to only reject EOS if we are flushing the decoder, and will add a comment. https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:330: weak_factory_.GetWeakPtr(), sequence_token_, On 2016/04/14 00:38:04, sandersd wrote: > If OnDecodeDone() should never be called when the sequence token is old, it may > be simpler to create a second WeakPtrFactory just for this Bind(). Then you can > call Invalidate() to prevent any more callbacks from coming. (You can still vend > new weak pointers from the same factory.) I did not know this was something that could be done. This is great, thank you :) https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:441: if (!pending_buffers_.empty() && On 2016/04/14 00:42:53, DaleCurtis wrote: > Multiline if needs {} CL format pushed it to multiline :(... Done. https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:483: if (!pending_buffers_.empty() && previous_decoder_) { On 2016/04/14 00:38:04, sandersd wrote: > It seems that there really needs to be two lists of pending buffers; one is what > has been fed in so far, the other is what has yet to be sent to the current > decoder. Then when switching decoders the second list can just be reset to a > copy of the first, and there are no strange conditions to check. > > It may be easiest to do that by splitting Decode() into two methods: a wrapper > that records to that list (Decode()), and an internal method that actually > dispatches (DoDecode() or DecodeInternal()). Then you can call the internal one > here and don't need to worry about the pending list being walked on. Good idea. Really simplifies the logic in many places, and adds support for multiple consecutive failures. https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:514: // Save valid buffers to be consumed by the new decoder On 2016/04/14 00:42:53, DaleCurtis wrote: > You'll need to set STATE_ERROR in the non-kOk case and adapt > OnDecoderReinitialized to handle it I think. Talked a bit offline with Dan. Created https://bugs.chromium.org/p/chromium/issues/detail?id=603713 https://codereview.chromium.org/1879353003/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:536: // Pending buffers might not match the reinitiliazed decoder's new config On 2016/04/14 00:42:53, DaleCurtis wrote: > DCHECK(pending_buffers_.empty()); ? I think this can't happen with your above > code. Also misspelled reinitialized. This is when we receive a normal config change before having outputted a frame, which can still happen I think. This is not when we receive a config change while reinitializing.
https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.cc:263: CompleteDecoderReinitialization(false); Sorry, I didn't take into account what should be done with |selected_decoder|. Looking into this now.
https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.cc:84: pending_buffers_.clear(); You don't actually need to do this, the destructor will handle it for you. https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.cc:343: // the EOS buffer we send to flush the decoder. I still think this would be more clear if ReadFromDemuxerStream() called a shared helper which would never push buffers into |pending_buffers_|. Right now it's not trivial to understand the exact usage of the two lists, nor the conditions under which they should be updated. https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.cc:394: // Prevent all pending decode requests from being called back. Newline before comment. https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.cc:464: if (!pending_buffers_.empty()) No need for a condition, it can just be unconditionally cleared. https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... File media/filters/decoder_stream.h (right): https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.h:250: base::WeakPtrFactory<DecoderStream<StreamType>> decode_weak_factory_; This should have a comment, since the usage pattern isn't common in media yet. Just explain that it will be invalidated each time we fall back to a new decoder, so that outstanding callbacks will be dropped.
https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.cc:84: pending_buffers_.clear(); On 2016/04/14 23:10:30, sandersd wrote: > You don't actually need to do this, the destructor will handle it for you. Done. https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.cc:263: CompleteDecoderReinitialization(false); On 2016/04/14 22:26:42, ThomasGuilbert wrote: > Sorry, I didn't take into account what should be done with |selected_decoder|. > Looking into this now. Done. https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.cc:343: // the EOS buffer we send to flush the decoder. On 2016/04/14 23:10:30, sandersd wrote: > I still think this would be more clear if ReadFromDemuxerStream() called a > shared helper which would never push buffers into |pending_buffers_|. Right now > it's not trivial to understand the exact usage of the two lists, nor the > conditions under which they should be updated. Done. After our offline discussion, I realized you probably meant FlushDecoder() instead of ReadFromDemuxerStream() (which was the source of my confusion). https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.cc:394: // Prevent all pending decode requests from being called back. On 2016/04/14 23:10:30, sandersd wrote: > Newline before comment. Done. https://codereview.chromium.org/1879353003/diff/40001/media/filters/decoder_s... media/filters/decoder_stream.cc:464: if (!pending_buffers_.empty()) On 2016/04/14 23:10:30, sandersd wrote: > No need for a condition, it can just be unconditionally cleared. Oops! Artifact of the previous patchset.
lgtm % nits. Nice tests! If you keep writing tests like that we're going to toss WebMediaPlayerImpl at you :p https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.cc:277: fallback_buffers_.insert(fallback_buffers_.begin(), Is it better to just insert at the end of pending_buffers_ and swap into fallback_buffers_ ? I forget what the efficiency of the deque is for front inserts. https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.cc:370: Delete? https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.cc:558: default: No need for default? We typically prefer to enumerate all cases so its caught as a warning when new cases are added w/o updates here. https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.cc:578: // Pending buffers might not match the reinitialiazed decoder's new config Reinitialized. https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... File media/filters/decoder_stream.h (right): https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.h:247: // Stores buffers that are garanteed to be fed to the decoder before fetching guaranteed https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.h:248: // more from the demuxer stream. Populated by |pending_buffers_| on decoder "Populated by ..." seems like it should be followed by a function name, not another variable name? https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.h:254: base::WeakPtrFactory<DecoderStream<StreamType>> decode_weak_factory_; This should have a comment about exactly what this is for. https://codereview.chromium.org/1879353003/diff/100001/media/filters/video_fr... File media/filters/video_frame_stream_unittest.cc (right): https://codereview.chromium.org/1879353003/diff/100001/media/filters/video_fr... media/filters/video_frame_stream_unittest.cc:813: return; Multiline if needs {}
Description was changed from ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=??? TEST=Ran media_unittests successfully ========== to ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=128837 TEST=Ran media_unittests successfully ==========
https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.cc:277: fallback_buffers_.insert(fallback_buffers_.begin(), On 2016/04/15 01:19:16, DaleCurtis_OOO_Until_Apr_20 wrote: > Is it better to just insert at the end of pending_buffers_ and swap into > fallback_buffers_ ? I forget what the efficiency of the deque is for front > inserts. While I assume that the amortized cost would be fine, it's probably better to do it the way you suggested it. https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.cc:370: On 2016/04/15 01:19:17, DaleCurtis_OOO_Until_Apr_20 wrote: > Delete? Done. https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.cc:558: default: On 2016/04/15 01:19:17, DaleCurtis_OOO_Until_Apr_20 wrote: > No need for default? We typically prefer to enumerate all cases so its caught as > a warning when new cases are added w/o updates here. Good to know. TY! https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.cc:578: // Pending buffers might not match the reinitialiazed decoder's new config On 2016/04/15 01:19:17, DaleCurtis_OOO_Until_Apr_20 wrote: > Reinitialized. Done. https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... File media/filters/decoder_stream.h (right): https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.h:247: // Stores buffers that are garanteed to be fed to the decoder before fetching On 2016/04/15 01:19:17, DaleCurtis wrote: > guaranteed Done. https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.h:248: // more from the demuxer stream. Populated by |pending_buffers_| on decoder On 2016/04/15 01:19:17, DaleCurtis wrote: > "Populated by ..." seems like it should be followed by a function name, not > another variable name? Done. https://codereview.chromium.org/1879353003/diff/100001/media/filters/decoder_... media/filters/decoder_stream.h:254: base::WeakPtrFactory<DecoderStream<StreamType>> decode_weak_factory_; On 2016/04/15 01:19:17, DaleCurtis wrote: > This should have a comment about exactly what this is for. Done. https://codereview.chromium.org/1879353003/diff/100001/media/filters/video_fr... File media/filters/video_frame_stream_unittest.cc (right): https://codereview.chromium.org/1879353003/diff/100001/media/filters/video_fr... media/filters/video_frame_stream_unittest.cc:813: return; On 2016/04/15 01:19:17, DaleCurtis_OOO_Until_Apr_20 wrote: > Multiline if needs {} Done.
Ping :)
https://codereview.chromium.org/1879353003/diff/160001/media/filters/decoder_... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/1879353003/diff/160001/media/filters/decoder_... media/filters/decoder_stream.cc:571: pending_buffers_.clear(); If the current decoder fails after this, we should enter state STATE_CONFIG_CHANGE_RECEIVED_WHILE_REINITIALIZING_DECODER. Should there be a flag we set instead of this clear?
https://codereview.chromium.org/1879353003/diff/160001/media/filters/decoder_... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/1879353003/diff/160001/media/filters/decoder_... media/filters/decoder_stream.cc:571: pending_buffers_.clear(); On 2016/04/19 21:12:19, sandersd wrote: > If the current decoder fails after this, we should enter state > STATE_CONFIG_CHANGE_RECEIVED_WHILE_REINITIALIZING_DECODER. Should there be a > flag we set instead of this clear? After trying to change the above mentioned state to a flag, I feel as though it added complexity to all possible interactions between OnDecoderReinitialized(), OnDecoderSelected(), OnDecodeOutputReady() and FlushDecoder(), without an easy way to signal why we were doing this. I reverted my changes, and propose deferring the changes until we address 603713. The only cases where there is a significant difference in behavior are: - Decoder returns a DECODE_ERROR during flushing (As is: fallback, losing some frames. With the flag, error out completely). - Decoder returns no DECODE_ERROR and no during flushing (As is: keep going, losing some buffers/frames if we fallback. With the flag: error out completely). Does this seem acceptable? Are there any significant issues that might arise from not error-ing out?
On 2016/04/20 01:13:18, ThomasGuilbert wrote: > https://codereview.chromium.org/1879353003/diff/160001/media/filters/decoder_... > File media/filters/decoder_stream.cc (right): > > https://codereview.chromium.org/1879353003/diff/160001/media/filters/decoder_... > media/filters/decoder_stream.cc:571: pending_buffers_.clear(); > On 2016/04/19 21:12:19, sandersd wrote: > > If the current decoder fails after this, we should enter state > > STATE_CONFIG_CHANGE_RECEIVED_WHILE_REINITIALIZING_DECODER. Should there be a > > flag we set instead of this clear? > > After trying to change the above mentioned state to a flag, I feel as though it > added complexity to all possible interactions between OnDecoderReinitialized(), > OnDecoderSelected(), OnDecodeOutputReady() and FlushDecoder(), without an easy > way to signal why we were doing this. I reverted my changes, and propose > deferring the changes until we address 603713. The only cases where there is a > significant difference in behavior are: > > - Decoder returns a DECODE_ERROR during flushing (As is: fallback, losing some > frames. With the flag, error out completely). > > - Decoder returns no DECODE_ERROR and no during flushing (As is: keep going, > losing some buffers/frames if we fallback. With the flag: error out completely). > > Does this seem acceptable? Are there any significant issues that might arise > from not error-ing out? What happens if we feed in four frames (CanDecodeMore() is now false), then get a config change, and then the first decode fails? Do we ever poll for more frames?
lgtm % output_cb should probably also be invalidated.
still lgtm
Description was changed from ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=128837 TEST=Ran media_unittests successfully ========== to ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=128837 TEST=Ran media_unittests successfully, manually verified that it works for MSE ==========
Description was changed from ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=128837 TEST=Ran media_unittests successfully, manually verified that it works for MSE ========== to ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=128837 TEST=Ran media_unittests successfully, manual (works as expected for MSE, bug 605790 for SRC=) ==========
The CQ bit was checked by tguilbert@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sandersd@chromium.org Link to the patchset: https://codereview.chromium.org/1879353003/#ps200001 (title: "new test for EOS flushing edge case")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1879353003/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1879353003/200001
Message was sent while issue was closed.
Description was changed from ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=128837 TEST=Ran media_unittests successfully, manual (works as expected for MSE, bug 605790 for SRC=) ========== to ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=128837 TEST=Ran media_unittests successfully, manual (works as expected for MSE, bug 605790 for SRC=) ==========
Message was sent while issue was closed.
Committed patchset #11 (id:200001)
Message was sent while issue was closed.
Description was changed from ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=128837 TEST=Ran media_unittests successfully, manual (works as expected for MSE, bug 605790 for SRC=) ========== to ========== Attempt decoder fallback if first decode fails This change is a manual merge of DaleCurtis' CL, along with unittests and and coverage for edge cases, such as handling parallel decode requests, config changes and EOS. Updated original description: There are several instances where a GPU video decoder will fail on the first decode even though it reported successfull initialization. In these cases try to fallback to a software decoder if possible. This works by queuing the first buffers sent to Decode() to a pending buffer queue, before any output callbacks have occurred. If decode fails before output is it will attempt to reselect a decoder. If this is successful the pending decode buffers will be copied to a fallback buffer queue, which will be fed in at an appropriate rate to the new decoder. Pending decode callbacks from the previous decoder are invalidated by way of a second weak_ptr factory for the decode callback. BUG=128837 TEST=Ran media_unittests successfully, manual (works as expected for MSE, bug 605790 for SRC=) Committed: https://crrev.com/e384d5d6be1ee2c196df5dc94c50419faf82f2a2 Cr-Commit-Position: refs/heads/master@{#388990} ==========
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/e384d5d6be1ee2c196df5dc94c50419faf82f2a2 Cr-Commit-Position: refs/heads/master@{#388990} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
