<div dir="ltr">For following comment, I think you meant that subtitle demux is not thread-safe. Because I tried to call demux_Control by the input control, which is called by the interface thread?<div><br></div><div>I know the concern here. Actually I already tried calling input_ControlPush with a command similar to INPUT_ADD_SUBTITLE/INPUT_CONTROL_ADD_SUBTITLE. But it failed because I want the control to return the time value. By calling input_ControlPush, the handling is asynchronous, so that the function call can not get the value right away. I don't know how to solve that. Not sure if there is any other example to return a value asynchronously?</div><div><br><span style="font-size:12.8000001907349px">> +</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +           if (!p_sub)</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +               return VLC_EGENERIC;</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +            int64_t* pstart = (int64_t*)va_arg( args, int64_t * );</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +            int64_t* pend = (int64_t*)va_arg( args, int64_t * );</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +            if (!demux_Control (p_sub->p_demux ,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +                    DEMUX_GET_SUBTITLE_CUR_</span><span style="font-size:12.8000001907349px">SENTENCE_TIME +</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +                    i_query - INPUT_GET_CUR_SENTENCE_TIME,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +                    pstart, pend))</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +                return VLC_SUCCESS;</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +            else</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">> +                return VLC_EGENERIC;</span><br style="font-size:12.8000001907349px"><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">This assumes that the subtitle demux is re-entrant - which it is not.</span><br style="font-size:12.8000001907349px"><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">Compare INPUT_ADD_SUBTITLE / INPUT_CONTROL_ADD_SUBTITLE,</span><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Mar 29, 2015 at 11:42 PM, Goldy Liang <span dir="ltr"><<a href="mailto:goldyliang@gmail.com" target="_blank">goldyliang@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>Hi Rémi,</div><div><br></div><div>Thanks for the response and it is very inspiring. Please see inline comments below and my new ideas.</div><div><br></div><div>Goldy</div><span class=""><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Date: Wed, 25 Mar 2015 19:08:55 +0200<br>
From: R?mi Denis-Courmont <<a href="mailto:remi@remlab.net" target="_blank">remi@remlab.net</a>><br>
To: <a href="mailto:vlc-devel@videolan.org" target="_blank">vlc-devel@videolan.org</a><br>
Subject: Re: [vlc-devel] [PATCH 1/2] Navigation enhancement for<br>
        language        learning<br>
Message-ID: <<a href="mailto:14274086.51sHASZh0G@basile.remlab.net" target="_blank">14274086.51sHASZh0G@basile.remlab.net</a>><br>
Content-Type: text/plain; charset="utf-8"<br>
<br>
> On Thu, Mar 19, 2015 at 6:13 AM, Goldy Liang <<a href="mailto:goldyliang@gmail.com" target="_blank">goldyliang@gmail.com</a>> wrote:<br>
> > I just had a glance how EsOutSend works, but I don't think it is good to<br>
> > implement there. Looks like EsOutSend would be called by all kinds of<br>
> > modules of demuxer or access. In EsOutSend, if I want to exact the time<br>
> > for subtitle, I have to check whether it is sent by the subtitle modules.<br>
<br>
I am not intimate with the ES output, but the type of the track, and its<br>
active status should be known there.<br>
<br>
> > And looks like each subtitle demux will send the data anyway no matter if<br>
> > it is selected or default.<br>
<br>
>From a quick look, the track is active if it has a non-NULL decoder_t pointer<br>
- but again, I am not an expert with es_out.c.<br>
<br>
It does not matter anyway. You can collect the timings for inactive tracks<br>
too. That way, seeking will work even if the user just changed the subtitle<br>
track. It is only when actually seeking that the input thread need to know<br>
which track is active, to query the timings from the correct track.<br></blockquote><div> </div></span><div>[Goldy]  If we want to seek a universal solution, yes EsOutSend may work. However looks like DecoderProcessSpu could be better. If my understanding is right, it should be called by any kinds of subtitle, then a specified decoder is called for different types by calling "p_dec->pf_decode_sub". (Correct me if I am wrong).</div><div><br></div><div>So the idea could be that, the decoder keeps and caches all the decoded subtitle timing information on the fly, so that the input thread is able to query the decoder module to get the timing of a prev/current sentence. Or even the timing of the next sentence (if it is ever decoded). The input thread could query the decoder to dump all cached subtitle data as well (for future use).</div><div><br></div><div>This solution should work for most (if not all) subtitles, even if it has the limitation like:</div><div>   - skip to next sentence may not work.</div><div>   - skip to previous sentence may not work either, if the user has just done a forward jump (information of prev sentence not known)</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
<br>
> > Even if we can identify the subtitle data and save it, it all depends how<br>
> > much data has been sent so as to navigate based on that.<br>
<br></span>
Yes. There would be a problem if a sub.itle demuxer sent a single subtitle as<span class=""><br>
multiple blocks. But then again, the ES output works with all file formats and<br>
all codecs.<br>
<br>
It will be very difficult to explain users that the feature only works with some<br>
subtitle formats but not all, if we go with the subtitle.c solution.<br></span></blockquote><div><br></div><div>[Goldy] It is definitely good if the function can work for all subtitle formats.</div><div><br></div><div>However besides working on the universal solution, I would still want to keep the subtitle.c</div><div>solution because the stand-alone text-based subtitle files have the least limitation for</div><div>subtitle based navigation, and I believe this kind of subtitle resources are very common</div><div>in the internet. So, if there is subtitle loaded by subtitle.c, totally perfect, if not, the universal</div><div>solution still works for most functions.</div><span class=""><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
> > For example, if the data of next sentence not sent yet, we can not jump<br>
> > forward one sentence; or if the user just changes the slider bar and jump<br>
> > to a specific position, we may be missing the time data for the previous<br>
> > sentence, etc. And another problem is where to save the time data and how<br>
> > the input thread can access it. In a word, it looks complicate and not<br>
> > good to me.<br>
<br>
Jumping forward is not possible as such with the ES output. Still fast forward<br>
would be feasible if a little difficult. Also, fast forwarding is usually more<br>
robust than seeking.<br></blockquote><div> </div></span><div>[Goldy] Doing a fast forward would be a good idea. It looks a bit hard but obviously workable.</div><div>Also, if a prev-sentence information is not there, a fast backward should also work.</div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
I would however think that seeking to previous and current subtitles is much<br>
more essential to the intended use case than seeking to the next subtitle.<br></blockquote><div><br></div></span><div>[Goldy] When the user wants to review a video clip for language study, seeking forward is a </div><div>useful function. </div><div><br></div><div>I think I can use the interface I mentioned above to dump the cached subtitle information to the interface</div><div>module, then the interface can save it in a standalone subtitle file which can be used later on for navigation</div><div>purpose.</div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
> > On the other hand, by controlling the demux, it is more simple and<br>
> > provides more flexibility for future development.<br>
<br>
Controlling the demux is likely simpler if you stick to subtitle.c. But the<br>
amount of changes in src/input/ as a whole is comparable if not larger.<br>
<br>
Still, the big problem with the demux approach is tht it will affect a lot of<br>
different demux plugins. On top of my head, besides subtitle, avformat, AVI,<br>
dvdnav, dvdread, MKV, MP4, MPEG-PS, MPEG-TS can contain subtitles, and that is<br>
not even exhaustive.<br>
<br>
> > I have more features in my mind about language learning, for example, we<br>
> > can show all subtitles in a separate window so that the user can glance<br>
> > and jump to any sentence he wants.  This requires the interface module to<br>
> > be able to access the whole subtitle data instead of just the time of<br>
> > prev/next sentence. I would like to make the subtitle demux to clone and<br>
> > dump the whole subtitle data through the input thread control to the<br>
> > interface. That would be consistent with the current mechanism that the<br>
> > subtitle demux reports the timestamps of prev/cur/next sentences to input<br>
> > thread then to interface. If we do that in es_out, I think it would not be<br>
> > possible to allow the interface to access the subtitle data freely.<br>
<br>
The least intrusive way to dump the whole subtitle track for display and/or<br>
random access ahead of time in a format-independent fashion is to run the<br>
demuxer in a tight loop with a purpose-specific es_out_t instance that just<br>
records all the subtitles for later use and discards everything else.<br>
<br>
An example of custom es_out_t is the Bluray plugin.<br></blockquote><div> </div></span><div>[Goldy] Dumping the subtitle data like your said would be a next-step for me.</div><div>However dumping the subtitle data loaded in subtitle.c would be more easy for</div><div>now.</div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
--<br>
R?mi Denis-Courmont<br>
<a href="http://www.remlab.net/" target="_blank">http://www.remlab.net/</a><br>
<br><br>
</blockquote></span></div><br></div></div>
</blockquote></div><br></div>