[FFmpeg-devel] [PATCH] Add -t option to ffplay

[Michael Niedermayer]

On Wed, Mar 31, 2010 at 05:15:25PM +0200, Robert Kr?ger wrote:
> 
> On 31.03.2010, at 16:06, Michael Niedermayer wrote:
> 
> > On Wed, Mar 31, 2010 at 01:52:52PM +0200, Robert Kr?ger wrote:
> >> 
> >> On 27.03.2010, at 00:00, Stefano Sabatini wrote:
> >> 
> >>> On date Wednesday 2010-03-24 16:58:59 +0100, Robert Kr?ger encoded:
> >>>> 
> >>>> On 24.03.2010, at 16:37, Michael Niedermayer wrote:
> >>>> 
> >>>>> On Wed, Mar 24, 2010 at 04:34:16PM +0100, Robert Kr?ger wrote:
> >>>>>> 
> >>>>>> On 24.03.2010, at 15:43, Michael Niedermayer wrote:
> >>>>>> 
> >>>>>>> On Wed, Mar 24, 2010 at 10:10:46AM +0100, Robert Kr?ger wrote:
> >>>>>>>> 
> >> 
> >>>>>>>> 
> >>> 
> >>> Missing manpage docs.
> >> 
> >> Oh, yes. Simply forgot.
> >> 
> >> Updated patch attached.
> >> 
> > [...]
> >> @@ -2473,6 +2476,8 @@
> >>             SDL_Delay(10);
> >>             continue;
> >>         }
> >> +        if(start_time != AV_NOPTS_VALUE)
> >> +            effective_start_time = start_time;
> >>         if(url_feof(ic->pb) || eof) {
> >>             if(is->video_stream >= 0){
> >>                 av_init_packet(pkt);
> >> @@ -2484,7 +2489,7 @@
> >>             SDL_Delay(10);
> >>             if(is->audioq.size + is->videoq.size + is->subtitleq.size ==0){
> >>                 if(loop!=1 && (!loop || --loop)){
> >> -                    stream_seek(cur_stream, start_time != AV_NOPTS_VALUE ? start_time : 0, 0, 0);
> >> +                    stream_seek(cur_stream, effective_start_time, 0, 0);
> >>                 }else if(autoexit){
> >>                     ret=AVERROR_EOF;
> >>                     goto fail;
> > 
> > why is this changed?
> > 
> 
> it is extracted into a variable because otherwise the conditional (start_time != AV_NOPTS_VALUE ? start_time : 0) would be used twice expressing the same thing.

i think using the conditional twice is more readable here than a distant
variable


> 
> > 
> >> @@ -2501,11 +2506,16 @@
> >>             SDL_Delay(100); /* wait for user event */
> >>             continue;
> >>         }
> >> -        if (pkt->stream_index == is->audio_stream) {
> >> +        /* check if packet is in play range specified by user, then queue, otherwise discard */
> >> +        pkt_past_play_range = duration != AV_NOPTS_VALUE &&
> >> +                (pkt->pts - ic->streams[pkt->stream_index]->start_time) *
> >> +                av_q2d(ic->streams[pkt->stream_index]->time_base) - (double)effective_start_time/1000000
> >> +                > ((double)duration/1000000);
> >> +        if (!pkt_past_play_range && pkt->stream_index == is->audio_stream) {
> >>             packet_queue_put(&is->audioq, pkt);
> >> -        } else if (pkt->stream_index == is->video_stream) {
> >> +        } else if (!pkt_past_play_range && pkt->stream_index == is->video_stream) {
> >>             packet_queue_put(&is->videoq, pkt);
> >> -        } else if (pkt->stream_index == is->subtitle_stream) {
> >> +        } else if (!pkt_past_play_range && pkt->stream_index == is->subtitle_stream) {
> >>             packet_queue_put(&is->subtitleq, pkt);
> >>         } else {
> >>             av_free_packet(pkt);
> > 
> 
> > it seems to me that a pkt_in_play_range would lead to simpler code
> 
> I was trying to avoid two calls to av_free_packet (one for the case that it is not in playable range and the other for the packet not being audio, video or subtitle). That's why I ended up with the && in each if. Do you have a suggestion with one call to av-free-packet and less complex conditionals or do you mean it would be sufficient to reformulate it as pkt_in_play_range and get rid of the three negations?  

i just meant the 3 negations
its just a small step but cleaner IMHO

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100331/e06ed417/attachment.pgp>