"Ronald S. Bultje" <[email protected]> writes:

> Hi,
>
> On Mar 27, 2011, at 3:28 PM, Vitor Sessak <[email protected]> wrote:
>
>> On 03/27/2011 08:47 PM, Måns Rullgård wrote:
>>> Vitor Sessak<[email protected]>  writes:
>>> 
>>>> On 03/27/2011 06:02 PM, Vitor Sessak wrote:
>>>>> On 03/27/2011 01:50 PM, Ronald S. Bultje wrote:
>>>>>> Hi,
>>>>>> 
>>>>>> On Sun, Mar 27, 2011 at 6:58 AM, Vitor Sessak<[email protected]>  
>>>>>> wrote:
>>>>>>> On 03/26/2011 08:15 PM, Ronald S. Bultje wrote:
>>>>>>>> 
>>>>>>>> Hi,
>>>>>>>> 
>>>>>>>> $subj. This is useful since with -mt enabled, order of packets
>>>>>>>> changes, which makes the original fate test fail.
>>>>>>> 
>>>>>>> I'm not against this patch, but how hard would it be to reorder
>>>>>>> packets so
>>>>>>> that if two packets have the same timestamp, to output the audio one
>>>>>>> first?
>>>>>> 
>>>>>> Not very difficult, I think. But is that correct?
>>>>> 
>>>>> For me it is not less correct than having no criteria at all, but better
>>>>> for regtesting in general. Maybe in the future other tests will fail for
>>>>> the same reason after non-buggy modifications.
>>>> 
>>>> And as a RFC, the patch to put video frames first (Note: this changes
>>>> a few fate references).
>>>> 
>>>> -Vitor
>>>> 
>>>> From 95307de0524ff4b7abaf457f8187af78f685460a Mon Sep 17 00:00:00 2001
>>>> From: Vitor Sessak<[email protected]>
>>>> Date: Sun, 27 Mar 2011 19:22:55 +0200
>>>> Subject: [PATCH] Order packets with identical PTS by packet type (video, 
>>>> audio, etc). This
>>>>  allows for more reproducible results when using multi-threading.
>>>> 
>>>> ---
>>>>  libavformat/utils.c |    5 ++++-
>>>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>>> 
>>>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>>>> index 7ece078..946e64e 100644
>>>> --- a/libavformat/utils.c
>>>> +++ b/libavformat/utils.c
>>>> @@ -3050,7 +3050,10 @@ static int 
>>>> ff_interleave_compare_dts(AVFormatContext *s, AVPacket *next, AVPacke
>>>>      AVStream *st2= s->streams[ next->stream_index];
>>>>      int64_t a= st2->time_base.num * (int64_t)st ->time_base.den;
>>>>      int64_t b= st ->time_base.num * (int64_t)st2->time_base.den;
>>>> -    return av_rescale_rnd(pkt->dts, b, a, AV_ROUND_DOWN)<  next->dts;
>>>> +    int64_t dts1 = av_rescale_rnd(pkt->dts, b, a, AV_ROUND_DOWN);
>>>> +    if (dts1 == next->dts)
>>>> +        return st->codec->codec->type<  st2->codec->codec->type;
>>>> +    return dts1<  next->dts;
>>>>  }
>>> 
>>> Wouldn't sorting by stream index be more robust?  Suppose you have two
>>> audio or two video streams...
>> 
>> The problem is that for some formats (like the typical game ones), the 
>> stream index is an arbitrary choice of the demuxer. So it could break when 
>> doing a demuxer cleanup. OTOH, something like the following might solve both 
>> problems:
>> 
>> +    int64_t dts1 = av_rescale_rnd(pkt->dts, b, a, AV_ROUND_DOWN);
>> +    if (dts1 == next->dts) {
>> +        if (st->codec->codec->type == st2->codec->codec->type)
>> +             return pkt->stream_index < next->stream_index;
>> +        else
>> +            return st->codec->codec->type < st2->codec->codec->type;
>> +    }
>> +    return dts1 < next->dts;

I don't see the point of sorting by type.  You still have the same
potential problem if a demuxer is changed to assign stream indexes to
same-type streams differently.  Moreover, I think having output order
affected by a change in a demuxer is acceptable.  Stability with respect
to unrelated features (e.g. threading) is what's important.

> This looks good to me, and my patch can then (optionally) be reverted.

Keeping the tests separate is still useful.  Now it's actually possible
to find the test for adpcm_xa without checking all the samples.

-- 
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to