[Spice-devel] [spice-gtk v1 3/4] channel-display-gst: improve check for decoder element

[Victor Toso]

Hi,

On Wed, Oct 19, 2016 at 12:11:53PM +0200, Francois Gouget wrote:
> On Tue, 18 Oct 2016, Victor Toso wrote:
> [...]
> > +    elements = g_strsplit(gst_opts[codec_type].dec_name, "!", 0);
>
> This would not work if one of the elements takes options.
> (not the case right now but if we can keep the option open)

Indeed.

>
> > +    for (i = 0; elements[i] != NULL; i++) {
> > +        GstElementFactory *factory;
> >
> > -    return has_codec;
> > +        factory = gst_element_factory_find(g_strstrip(elements[i]));
> > +        if (factory == NULL) {
> > +            SPICE_DEBUG("no element %s", elements[i]);
> > +            g_strfreev(elements);
> > +            return FALSE;
> > +        }
>
> This will likely not be an issue with software decoders but with
> hardware ones that approach will not guarantee that the decoder is
> usable.

That's correct but I think the same apply for the current code in regard
of hw decoders - We might be able to create the pipeline at this time
without errors but failed later on when we have a stream to decode.

IMHO, we will need a whole different approach for hardware decoder.

> In a different context I have found that I can fnid dfbvideosink and
> even instantiate it. But it will refuse to switch to the READY state and
> is thus unusable. That makes sense since I was not using it in a Direct
> FB context.

Interesting. Was it possible to set a dfb context later on? Otherwise it
could be a bug, no?

> I expect hardware decoders will be even worse. For instance vaapidecode
> is likely to be present but may not work if you don't have an Intel
> card. Even if you can instantiate it, it may cause a VP8 pipeline caps
> negotiation to fail if your graphics does not support VP8.

Yes, that's why I think we need a different approach for hardware
decoders. We should have an api in gstreamer-vaapi to tell what we can
hw decode, I think (somewhat, a parser for vainfo command + validation
with GStreamer vaapi elements).

> Given that this code is not speed critical I don't think there's much
> point in making it more complex and likely less reliable.

True, I don't really mind to drop it but I don't find it less reliable
for software decoders and we don't have the correct handling for
hardware decoders yet. So, all in all for me it is an improvement.

Thanks for your review!
  toso
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20161019/7771561b/attachment.sig>