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

[Robert Krüger]

On 31.03.2010, at 21:29, Michael Niedermayer wrote:

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

OK, changed.

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

You're right. Changed.

Regards,

Robert

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffplay-duration-option.patch
Type: application/octet-stream
Size: 3177 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100401/06f7ac2b/attachment.obj>