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

[Robert Krüger]

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.

> 
>> @@ -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?  

Regards,

Robert