<br><br><div class="gmail_quote">On Tue, Feb 23, 2010 at 9:02 PM, Laurent Aimar <span dir="ltr"><<a href="mailto:fenrir@via.ecp.fr">fenrir@via.ecp.fr</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi,<br>
<div class="im"><br>
On Mon, Feb 22, 2010, Pierre d'Herbemont wrote:<br>
> Hey,<br>
> Please review the attached patch.<br>
> I consider it important because it properly fix libvlc_media_get_es(). We<br>
> are currently advising our poor user to create a new media_player with<br>
> --sout=#description which makes us look overcomplicated. Mostly because<br>
> you still have to attach an event and such to know when we are done, and<br>
> also because a full playback is launched, not saving CPU and memory.<br>
> Our alternative to this patch is to read the input_item's info dictionary.<br>
> And parse the string in it. Hence the modification stay in libvlc umbrella<br>
> and not in the core.<br>
> However I consider this patch as low risk:<br>
> - It is following how we fill the input_item's info which is an already<br>
> existing string dictionary.<br>
> - Its impact in term of performance is low. We are not using a full<br>
> es_format_t, but just a stripped down version of fixed size, which is what<br>
> we expose in libvlc_media.<br>
> - It's chirugical and circumvented.<br>
> - It's fixing a real issue in libvlc.<br>
</div> Well, it's adding a feature not fixing an issue (I don't say that it's not a<br>
wanted one).</blockquote><div><br></div><div>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... :)</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="im">
> If it's not accepted I'll fix libvlc_media_get_es() in 1.1 by parsing<br>
> input_item's info field, which are correctly filled after preparsing for<br>
> most demuxer. I am ok with this, but wanted to get the patch archived on<br>
> the ML at least.<br>
</div>Except the following remarks, I am ok with the way it is implemented.<br></blockquote><div><br></div><div>ok. I've added some remarks, just to let you know. Rest will be fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
> diff --git a/include/vlc_input_item.h b/include/vlc_input_item.h<br>
> index d4858c9..1e10acd 100644<br>
> --- a/include/vlc_input_item.h<br>
> +++ b/include/vlc_input_item.h<br>
> @@ -51,6 +51,44 @@ struct info_category_t<br>
> struct info_t **pp_infos; /**< Pointer to an array of infos */<br>
> };<br>
><br>
> +/* Stripped down version of es_format_t because es_format_t can become very<br>
> + * large in term of memory. This is mostly libvlc_media_es_t */<br>
> +typedef struct input_item_es_t {<br>
Usually useless to name the struct part.<br></blockquote><div><br></div><div>well, it produces nicer error with most C compilers (including gcc) because upon warning it will name it "anonymous struct".</div><div>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Also, current item->es is not of the right type as it is a es_format_t (I haven't<br>
saw any change to it).</blockquote><div><br></div><div>oh. Weird, was here but seems to be forgotten. It was the "es" field being reused.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Changing the type of item->es will make #describe unusable,</blockquote><div><br></div><div>Yes.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"> so either we need to add new fields to input_item_t or breaks the VLM VoD part.</blockquote>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Btw, you also need to clean up the array when destructing the input_item_t.<br></blockquote><div> </div><div>Because we need to converge, I'll add a new field called "track info" (to match libvlc) and put it here.</div>
<div><br></div><div>Thanks,</div><div><br></div><div>Pierre.</div><div><br></div></div>