Created attachment 344908 [details] [review]
parsebin: Don't expose endpad if corresponding decoder is not supported

Remove the endpad if decoder is missing before parsebin is exposing all endpads.

Created attachment 344909 [details]
Dot graph for playbin3 with missing audio decoder in fixed version

Dot graph for playbin3 with missing audio decoder in fixed version

Created attachment 344910 [details] [review]
gst-play: make unsupported decoder by adjusting rank

If you use make_unsupported_codec () function, you can adjust rank of decoder what you don't want to support as NONE.
 
Thanks.

This will be quite unfortunate if the sink supports that format (like passthrough HDMI) but there is no decoder present.

Also this will totally break the behaviour of parsebin outside of decodebin3. One might want to use parsebin for re-muxing purposes for example.

I would say the solution here is to (finally) implement the signals that were removed from db3/pb3 : https://bugzilla.gnome.org/show_bug.cgi?id=769079

(In reply to Edward Hervey from comment #4)
> This will be quite unfortunate if the sink supports that format (like
> passthrough HDMI) but there is no decoder present.
> 
> Also this will totally break the behaviour of parsebin outside of
> decodebin3. One might want to use parsebin for re-muxing purposes for
> example.
> 
> I would say the solution here is to (finally) implement the signals that
> were removed from db3/pb3 : https://bugzilla.gnome.org/show_bug.cgi?id=769079

As general purpose for parsebin, I agree with your opinion.
 
Let's pretend that there is a stream 1 video and 1 audio but, we can not find available decoder for the audio.
When playbin2, we can play video without audio.
But, negotiation failure is happened and can not play in playbin3.
So, I think that most developers and end users may recognize this is a kind of side effect or regression compared with playbin2.
Also, they want to play without audio in playbin3.
  
  
1) Can you acceptable my approach if I add new property likes 'needs-decoder' in parsebin and then checking expose endpad or not?
  
2) I am not sure I can cope unsupported decoder after I implement the removed signals from https://bugzilla.gnome.org/show_bug.cgi?id=769079.
Can I get a hint how to handle it with these signals?
 
Below is my observations after non available audio stream is exposed in parsebin.
First one, we may have to handle then negotiation failure problem by Accept Caps query from gstdecodebin3-parse.c.
Second, we may have to synchronize between available and non-available audio stream by clock(e.g. running time) and drop buffers likes MQ or input-selector if non-available audio stream is exposed in parsebin.
Third, we have have to handle preroll state by sending gap event when transiting paused state if non-available audio stream is exposed in parsebin.
 
It would be very helpful if either my patch would be acceptable or you give a hint to use these removed signals as well.
 
Thanks.

Shouldn't playbin give parsebin also the caps of the sink, then the check for "decoder" should also check for a sink too?

Something like: ((IS_FACTORY (x) && (gst_element_factory_list_is_type(x, .._DECODER) || gst_element_factory_list_is_type(x, .._SINK))) || (IS_ELEMENT(x) && GST_ELEMENT_IS_SINK(x)))

(In reply to Olivier Crête from comment #6)
> Shouldn't playbin give parsebin also the caps of the sink, then the check
> for "decoder" should also check for a sink too?
> 
> Something like: ((IS_FACTORY (x) && (gst_element_factory_list_is_type(x,
> .._DECODER) || gst_element_factory_list_is_type(x, .._SINK))) ||
> (IS_ELEMENT(x) && GST_ELEMENT_IS_SINK(x)))

Hello Olivier Crête
Sorry, I don't get it what you mean exactly.
You want me to use autoplug-{factories/select/continue} signals in playbin3?
It would be helpful if you give me detailed explanation.
 
Thanks.

Yes, I think that when we want to add support for decoding sinks in playbin3, we'll to have parsebin delegate the decision on whether this is an acceptable caps to expose or not to the parent playbin3 which knows about the sinks.

(In reply to Olivier Crête from comment #8)
> Yes, I think that when we want to add support for decoding sinks in
> playbin3, we'll to have parsebin delegate the decision on whether this is an
> acceptable caps to expose or not to the parent playbin3 which knows about
> the sinks.

Hello Olivier Crête
Thanks for your reply.
 
I have one more question.
In latest version, parsebin emits "autoplug-factories" to decodebin3 in analyze_new_pad () but, decodebin3 does not add callback function likes "g_signal_connect (parsebin, "autoplug-factories"...)".
Thus, decodebin3 can not response the "autoplug-factories" signal and then
can not emit "autoplug-factories" signal to playbin3.
 
Is there any intention not to implement "autoplug-factories" signal between
decodebin3 and playbin3? or just missed to implement?
  
Thanks.

(In reply to Olivier Crête from comment #6)
> Something like: ((IS_FACTORY (x) && (gst_element_factory_list_is_type(x,
> .._DECODER) || gst_element_factory_list_is_type(x, .._SINK))) ||
> (IS_ELEMENT(x) && GST_ELEMENT_IS_SINK(x)))

  You would need to instantiate the sink (potentially with the correct device id) to know exactly what the sink supports in terms of caps.

(In reply to HoonHee Lee from comment #9)
> I have one more question.
> In latest version, parsebin emits "autoplug-factories" to decodebin3 in
> analyze_new_pad () but, decodebin3 does not add callback function likes
> "g_signal_connect (parsebin, "autoplug-factories"...)".
> Thus, decodebin3 can not response the "autoplug-factories" signal and then
> can not emit "autoplug-factories" signal to playbin3.
>  
> Is there any intention not to implement "autoplug-factories" signal between
> decodebin3 and playbin3? or just missed to implement?

  The reason this was removed or deactivated is because we want to use a more correct/streamlined signal for this (as mentionned in the review signals bug).

  One idea could be the following:
  1) parsebin always exposes everything (regardless of decoder being present or not)
  2) streams go through multiqueue
  3) When deciding whether to use a decoder or not (downstream of multiqueue), fire the new signal.

  This signal would provide:
    * The GstStream object
    * The list of decoders (if any) that decodebin3 will try to link

  The callback can return:
    * The re-sorted list of decoders (potentially with new elements in it)
    * Whether the stream should:
      1) Be exposed as-is (without any decoder)
      2) Be discarded (never exposed, i.e. blacklisted, until the stream changed)
      3) Try to plug in the decoders in the provided order.

  This is the same thing as what was done previously. But just with one signal instead of 4.

  This should solve your problem as far as I understand.

(In reply to Edward Hervey from comment #10)
> (In reply to Olivier Crête from comment #6)
> > Something like: ((IS_FACTORY (x) && (gst_element_factory_list_is_type(x,
> > .._DECODER) || gst_element_factory_list_is_type(x, .._SINK))) ||
> > (IS_ELEMENT(x) && GST_ELEMENT_IS_SINK(x)))
> 
>   You would need to instantiate the sink (potentially with the correct
> device id) to know exactly what the sink supports in terms of caps.
> 
> (In reply to HoonHee Lee from comment #9)
> > I have one more question.
> > In latest version, parsebin emits "autoplug-factories" to decodebin3 in
> > analyze_new_pad () but, decodebin3 does not add callback function likes
> > "g_signal_connect (parsebin, "autoplug-factories"...)".
> > Thus, decodebin3 can not response the "autoplug-factories" signal and then
> > can not emit "autoplug-factories" signal to playbin3.
> >  
> > Is there any intention not to implement "autoplug-factories" signal between
> > decodebin3 and playbin3? or just missed to implement?
> 
>   The reason this was removed or deactivated is because we want to use a
> more correct/streamlined signal for this (as mentionned in the review
> signals bug).
> 
>   One idea could be the following:
>   1) parsebin always exposes everything (regardless of decoder being present
> or not)
>   2) streams go through multiqueue
>   3) When deciding whether to use a decoder or not (downstream of
> multiqueue), fire the new signal.
> 
>   This signal would provide:
>     * The GstStream object
>     * The list of decoders (if any) that decodebin3 will try to link
> 
>   The callback can return:
>     * The re-sorted list of decoders (potentially with new elements in it)
>     * Whether the stream should:
>       1) Be exposed as-is (without any decoder)
>       2) Be discarded (never exposed, i.e. blacklisted, until the stream
> changed)
>       3) Try to plug in the decoders in the provided order.
> 
>   This is the same thing as what was done previously. But just with one
> signal instead of 4.
> 
>   This should solve your problem as far as I understand.

Hello Edward Hervey
Thanks for your detailed guide.
I understood most of things that you suggested.
 
For 2), You want to maintain the GstStream which is not exposed in current collection? Or Removing the GstStream in current collection and posting it?
I think it is important to show either all tracks or supported tracks to player side.
  
Please feedback about my question.
Thanks.
 
Thanks.

(In reply to HoonHee Lee from comment #11)
> >   The callback can return:
> >     * The re-sorted list of decoders (potentially with new elements in it)
> >     * Whether the stream should:
> >       1) Be exposed as-is (without any decoder)
> >       2) Be discarded (never exposed, i.e. blacklisted, until the stream
> > changed)
> >       3) Try to plug in the decoders in the provided order.
> > 
> For 2), You want to maintain the GstStream which is not exposed in current
> collection? Or Removing the GstStream in current collection and posting it?
> I think it is important to show either all tracks or supported tracks to
> player side.

  Good point.

  One could argue that since something *outside* of playbin3 decided to blacklist a stream, it shouldn't appear in the resulting collection (which btw, would have to be re-posted by playbin3).

  Let me think about it a bit more and collect some other opinions.

Another argument that popped up is the following: Even if the stream is not exploitable by a player/app .. one might still want to be able to list it in the application (and maybe have it greyed out).

(In reply to Edward Hervey from comment #13)
> Another argument that popped up is the following: Even if the stream is not
> exploitable by a player/app .. one might still want to be able to list it in
> the application (and maybe have it greyed out).

Thanks for collecting another opinion.
I agree with this behaviour. Because, I think it's more general concept.
  
Eventually, decodebin3(playbin3) would switch track between available streams(tracks) and we don't need to worry about handling preroll state(async-done) when non-available stream(track) is selected and pipeline tries to change paused state.
  
Do you agree with following behaviour if you also want to maintain all streams?
   
1) If we have a stream that contains 1-video and 2-audios and 1st audio stream is not available(not supported decoder or in blacklist).
How about changing automatically from 1st audio to 2nd audio when initial pre-rolling? I think that changing automatically is more better instead of mute sound.
  
Thanks.

(In reply to HoonHee Lee from comment #14)
> 1) If we have a stream that contains 1-video and 2-audios and 1st audio
> stream is not available(not supported decoder or in blacklist).
> How about changing automatically from 1st audio to 2nd audio when initial
> pre-rolling? I think that changing automatically is more better instead of
> mute sound.

  For this use-case, the way I see it happening should be:
  * The 1st audio stream is initially selected automatically
  * When that 1st audio stream arrives downstream of multiqueue, decodebin3 fires the 'stream-handling' signal (or whatever we'll call it) with no available decoder factories
  * playbin3 can't handle it, returns that it does not want to have it exposed
  * decodebin3 marks it blacklisted for now, and re-triggers the initial selection (update_requested_selection) which would avoid using that stream (in the same way that we sort by SELECT/UNSELECT streams) => The 2nd audio stream ends up being selected instead.
  * decodebin3 triggers the stream switch update (it sees the current stream it wanted to expose isn't selected, and switches)

  That would solve the issue of exposing a stream that can't be handled

  It leaves the issue of:
  * What to do if the app decide to select that stream. How do we notify the user/app ? Do we still expose it if it was requested ?

I have a better and more generic solution for this. Keeping the bug open for reference.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/334.