[FFmpeg-devel] [PATCH] fix stream copy

[Michael Niedermayer]

On Mon, Feb 01, 2010 at 01:04:02PM -0800, Baptiste Coudurier wrote:
> On 02/01/2010 12:07 PM, Michael Niedermayer wrote:
>> On Mon, Feb 01, 2010 at 11:37:57AM -0800, Baptiste Coudurier wrote:
>>> On 02/01/2010 11:16 AM, Michael Niedermayer wrote:
>>>> On Mon, Feb 01, 2010 at 07:53:54PM +0100, Michael Niedermayer wrote:
>>>>> On Fri, Jan 29, 2010 at 03:01:20PM -0800, Baptiste Coudurier wrote:
>>>>>> On 01/27/2010 01:47 AM, Maksym Veremeyenko wrote:
>>>>>>> Baptiste Coudurier ???????(??):
>>>> [...]
>>>>>
>>>>>>
>>>>>> With your patch the definition would become wrong in some cases.
>>>>>>
>>>>>>>>
>>>>>>>> One other possible solution is to use pts.val when stream copy is
>>>>>>>> used.
>>>>>>>>
>>>>>>> it will require changing another part of code where ost->sync_opts 
>>>>>>> used
>>>>>>> for stop condition or keep previous packet pts to calc duration of
>>>>>>> previous packet....
>>>>>>>
>>>>>>
>>>>>> Hummm, I'm not sure what you mean, but this seems to fix the issue for
>>>>>> me.
>>>>>> Can you confirm ?
>>>>>>
>>>>>> The idea is to only use sync_opts if the codec time base of the output
>>>>>> looks like a frame rate.
>>>>>>
>>>>>> Michael what do you think ? I'm not sure why video is treated
>>>>>> separately.
>>>>>
>>>>> i think audio should use sync_opts as well, but this will need some 
>>>>> minor
>>>>> changes i suspect
>>>>
>>>> changed my mind, is there a reason why you did not just drop the 
>>>> sync_opts
>>>> case for video andmake it use what audio does?
>>>> ill probably commit that very soon unless i notice some breakage
>>>
>>> No, I already suggested that. That's why I asked you why the video case 
>>> was
>>> handled differently.
>>
>> it breaks the reg tests it seems
>
> It seems there is one frame too much in the output because opts.val is the 
> current value while sync_opts is the next value.

pts.val is the next dts, dts with B frame reordering is 1 off from what the
end detection code excpects, 2 with b pyramid and can be more.
pts should be renamed to dts but its public API

either way except the end check the code looks like it expects the dts
actually. Iam planing to put the end check a little later where we
have the true pts 
this fixes a problem with ogg encoding in the regression tests, as
our reference ogg file is too long

after that ill change video to pts.val well that is if tests pass

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- 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/20100201/75556ba7/attachment.pgp>