[vlc-devel] [Patch] Correctly fill out input_item's es info, and slim down input_item_es_t.

[Pierre d'Herbemont]

On Tue, Feb 23, 2010 at 9:02 PM, Laurent Aimar <fenrir at via.ecp.fr> wrote:

> Hi,
>
> On Mon, Feb 22, 2010, Pierre d'Herbemont wrote:
> >    Hey,
> >    Please review the attached patch.
> >    I consider it important because it properly fix libvlc_media_get_es().
> We
> >    are currently advising our poor user to create a new media_player with
> >    --sout=#description which makes us look overcomplicated. Mostly
> because
> >    you still have to attach an event and such to know when we are done,
> and
> >    also because a full playback is launched, not saving CPU and memory.
> >    Our alternative to this patch is to read the input_item's info
> dictionary.
> >    And parse the string in it. Hence the modification stay in libvlc
> umbrella
> >    and not in the core.
> >    However I consider this patch as low risk:
> >    - It is following how we fill the input_item's info which is an
> already
> >    existing string dictionary.
> >    - Its impact in term of performance is low. We are not using a full
> >    es_format_t, but just a stripped down version of fixed size, which is
> what
> >    we expose in libvlc_media.
> >    - It's chirugical and circumvented.
> >    - It's fixing a real issue in libvlc.
>  Well, it's adding a feature not fixing an issue (I don't say that it's not
> a
> wanted one).


I know it seems a bit artificial given how late this was implemented, but
the libvlc test about track_info is failing without this patch. I'll stop
the troll here... :)

>    If it's not accepted I'll fix libvlc_media_get_es() in 1.1 by parsing
> >    input_item's info field, which are correctly filled after preparsing
> for
> >    most demuxer. I am ok with this, but wanted to get the patch archived
> on
> >    the ML at least.
> Except the following remarks, I am ok with the way it is implemented.
>

ok. I've added some remarks, just to let you know. Rest will be fixed.


>
> > diff --git a/include/vlc_input_item.h b/include/vlc_input_item.h
> > index d4858c9..1e10acd 100644
> > --- a/include/vlc_input_item.h
> > +++ b/include/vlc_input_item.h
> > @@ -51,6 +51,44 @@ struct info_category_t
> >      struct info_t **pp_infos;     /**< Pointer to an array of infos */
> >  };
> >
> > +/* Stripped down version of es_format_t because es_format_t can become
> very
> > + * large in term of memory. This is mostly libvlc_media_es_t */
> > +typedef struct input_item_es_t {
>  Usually useless to name the struct part.
>

well, it produces nicer error with most C compilers (including gcc) because
upon warning it will name it "anonymous struct".


>  Also, current item->es is not of the right type as it is a es_format_t (I
> haven't
> saw any change to it).


oh. Weird, was here but seems to be forgotten. It was the "es" field being
reused.


> Changing the type of item->es will make #describe unusable,


Yes.


>  so either we need to add new fields to input_item_t or breaks the VLM VoD
> part.

 Btw, you also need to clean up the array when destructing the input_item_t.
>

Because we need to converge, I'll add a new field called "track info" (to
match libvlc) and put it here.

Thanks,

Pierre.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100223/d2a0b3ce/attachment.html>