Directshow VP8 decoder should discard all non key frames when starting up

[GoogleCodeExporter]

If the first sample provided to the Directshow filter is a non key frame the 
filter graph is stopped since the decoder return VPX_CODEC_UNSUP_BITSTREAM.

The situation - starting with a non key frame can legitimately happen in 
multiple scenarios including when using the decoder as a part of the a graph 
which starts with a network source and the client is joining mid stream.

One way of handling the case is to drain all non key frames when the filter 
starts up by discarding them or sending a green\black frame instead as the 
decoder input.

Original issue reported on code.google.com by howardcc...@gmail.com on 15 Mar 2013 at 5:24

[GoogleCodeExporter]

BTW this seems like a subset of #337 if 337 were fixed this would automatically 
be taken care of.

Original comment by howardcc...@gmail.com on 15 Mar 2013 at 5:28

[GoogleCodeExporter]

When the filter starts, it should handle non-key frames.  

One way to implement this might be to not transition out of the paused state 
until a keyframe is received by the filter.

Original comment by matthewj...@google.com on 15 Mar 2013 at 6:08

[GoogleCodeExporter]

Original comment by matthewj...@google.com on 15 Mar 2013 at 6:08

• Added labels: Component-DirectShow

[GoogleCodeExporter]

Pausing till key frame received might do the trick but I think there is the off 
chance that some allocator upstream runs out of buffers in the paused graph 
before a key frame is received and samples are no longer delivered to the 
decoder in which case there is no way to transition out of the paused state.

Original comment by howardcc...@gmail.com on 15 Mar 2013 at 6:59

[GoogleCodeExporter]

I'm not following this argument.  The decoder filter isn't holding onto (input) 
samples it receives from its upstream filter -- it's testing the keyframe 
status and immediately disposing of the sample if the keyframe status is false, 
keeping its state as Paused.  If the keyframe status is true, it decompresses 
the frame, disposes of the input sample, emits an output sample, and sets its 
own state to Running.

Original comment by matthewj...@google.com on 15 Mar 2013 at 7:10

[GoogleCodeExporter]

This was my chain of thought.

Assume the filter graph ends with a video renderer. At start up the graph 
manager pauses all filters. The final video renderer will receive exactly _one_ 
sample in the paused state and block (maybe displaying the sample as a poster 
image). If the graph remains paused for a long time one of two things happens 
either every filters ends up being blocked in the receive or on "allocator get 
buffer". There is a bounded number of sample buffers depending on allocator 
properties overall across the graph.

I can try and modify the sdk ezrgb sample so that it never transitions out from 
the paused state and see if it keeps getting samples from the upstream filter.

Original comment by howardcc...@gmail.com on 15 Mar 2013 at 7:59

[GoogleCodeExporter]

But the only filter (in your example) that blocks during the paused state is 
the video renderer.  While paused (or running) of the upstream filters simply 
push samples downstream.  (The paused vs. running distinction is only useful 
for video renderers.)

Original comment by matthewj...@google.com on 15 Mar 2013 at 9:12

[GoogleCodeExporter]

You are right.

I created a filter which takes forever to transition from paused to running and 
it continuously receives samples in its "switching to running" state.

I was wrongly assuming that the graph manager would not run the downstream 
filters till the filter completed it's switch to running - but that is not the 
case.

So your suggestion of staying paused and not transitioning to running till you 
receive a key frame should do the trick.

Original comment by howardcc...@gmail.com on 15 Mar 2013 at 9:15

[GoogleCodeExporter]

One solution is to create an internal state enumeration type, instead of using 
FILTER_STATE, comprising an additional enumeration literal to indicate 
"transitioning from stopped to paused".  While in that state, the filter throws 
away frames that aren't keyframes.

Original comment by matthewj...@google.com on 15 Mar 2013 at 9:29

[GoogleCodeExporter]

I was thinking about it and it is either that or another sub state variable - 
switching to XX state. I prefer the the new filter state as you also suggested 
since the state value is not misleading.

Could it also be from "transitioning from paused to running" as opposed to  
"transitioning from stopped to paused"?
If so why I can't think why one would better than the other.

When in the transitioning state drop all non key frames - once a key frame is 
hit finish the transition by moving to the actual FILTER_STATE state.
So the state transition happens in the middle of receive.

Also while in the the transitioning state get state returns indeterminate.

Original comment by howardcc...@gmail.com on 15 Mar 2013 at 9:38

[GoogleCodeExporter]

Can you depend solely on the incoming samples IsSyncPoint to discard non key 
frames or do you actually need to inspect the payload?

Original comment by howardcc...@gmail.com on 15 Mar 2013 at 9:42

[GoogleCodeExporter]

I think it's only "stopped to paused" that we need to handle.  There's nothing 
special we need to do when the graph is paused (and later un-paused) after it 
has already been running.

Original comment by matthewj...@google.com on 15 Mar 2013 at 9:43

[GoogleCodeExporter]

IsSyncPoint should correspond exactly to keyframe status.  There is no need to 
inspect payload directly.

Original comment by matthewj...@google.com on 15 Mar 2013 at 9:45

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

A second "Transitioning to Running" state might be required.

For example in the case:

Filter Stopped => Pause Request (Transitioning to Pause - since no key frame 
seen) => Run Request (This should still keep discarding non key frames - 
otherwise the user still gets an error).

With the 5 state implementation - The pause and Run both sometimes return 
S_FALSE to signal that the state transition is incomplete
and Get State needs to sometime return indeterminate.

The state transitions are more complicated from the simple filter state model 
where you can go from any state to any state.
If we keep the filter states and add a draining flag it might be a little more 
obvious to the reader what is happening.

Negatives of the draining flag is that the state is now smeared across two 
variables. and we need to guard against some nonsensical combinations (Running 
and draining).

Thoughts?

Original comment by howardcc...@gmail.com on 18 Mar 2013 at 7:32

[GoogleCodeExporter]

Right, I came to a similar conclusion this weekend.  There needs to be some 
mechanism for knowing that a run request was received while we were waiting for 
a keyframe.

Original comment by matthewj...@google.com on 18 Mar 2013 at 7:48

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I ended up making the change this morning. I defined two additional filter 
states; Paused_Waiting_For_Keyframe and Running_Waiting_For_Keyframe.

What would be the best way to get these merged. Does this issue need to show up 
in Gerrit first?

Original comment by howardcc...@gmail.com on 19 Mar 2013 at 2:11

[GoogleCodeExporter]

You need to push the changes up for review.  Use the push URL listed here:

http://www.webmproject.org/code/

Specify me as a reviewer, and I'll review your changes.

Original comment by matthewj...@google.com on 19 Mar 2013 at 7:37

[GoogleCodeExporter]

Original comment by albe...@google.com on 28 Mar 2013 at 10:20

• Changed state: Started

[GoogleCodeExporter]

I just noticed the status change. I am waiting on the Contributor Agreement 
from Legal - I should have it today or the coming Monday. Do you still want me 
to push the commit?

Original comment by howardcc...@gmail.com on 29 Mar 2013 at 3:42

[GoogleCodeExporter]

Changed was checked into repository on 2013/10/29:

https://gerrit.chromium.org/gerrit/gitweb?p=webm/webmdshow.git;a=commit;h=08a4a9
1ca9650b5b58008fac521f59eb518d929f

Original comment by matthewj...@google.com on 29 Oct 2013 at 10:15

• Changed state: Fixed