On Sat, Mar 10, 2018 at 03:08:16PM +0200, Jan Ekström wrote: > Hi, > > I would like to raise a question regarding how the community feels > about removing hacks which most likely were done to work around > deficiencies of API clients. Silently.
Iam in favor of removing hacks and replacing them by better solutions. To do this its important to first understand why a hack was needed and what it does. Otherwise no correct or better implementation is possible and removial would just add back a bug. > > Some weeks ago someone asked on #ffmpeg why he wasn't getting the time > scale (time base in MOV/ISOBMFF speak) he wanted out of movenc.c. So I > looked into it: > > 1) > https://git.videolan.org/?p=ffmpeg.git;a=commit;h=b02493e47668e66757b72a7163476e590edfea3a > (2012) Some API clients didn't clearly set a time base correctly for > VFR content - a hack was added in movenc.c to bump the time scale to > at least 10k. Iam not sure this was related to VFR as you write here. I think what this was about was A/V desync on more than VFR videos to illustrate, consider that if you create a slide show with 1fps you get upto 1 second desync if you are unlucky. the same happens with normal frame rates in CFR video. That is this fixed the case where a audio and video sample needed to be synced exactly and not by upto 1/30 only (for 30fps video) This problem was IIRC interlinked with the need to shift initial negative DTS into the zero or positive. So even if an application supplied exact timestamps in a low timebase like 1/30. after the shift to get DTS after 0 they would no longer be where they where before when the timebases where not accurate enough. So in from the applications point of view 1/30 could be fully exact and good enough yet because dts where negative and needed to be shifted a more precisse timebase was needed to do this without introducing AV desync. This issue may have been fixed with edit lists > 2) https://git.videolan.org/?p=ffmpeg.git;a=commit;h=7e570f027b9bb0b5504 > ed443c70ceb654930859d > (2013) Someone clearly didn't like this forced behavior so an > AVOption was added to set a forced video time base for ALL video > tracks in a mux. > 3) > https://git.videolan.org/?p=ffmpeg.git;a=commit;h=194be1f43ea391eb986732707435176e579265aa > (2014) Anton switches the time base utilized from the encoder > AVCodec to the AVStream, which most likely fixes the issue of output > time base being incorrect as some encoders dislike weird (?) time > bases (although how would you get VFR content through those if that > was the reason is still beyond me). > > After this I told the user that this is a hack that's been added and > that his timestamps are still exact (which they are, just that the > time base is larger than it is). But as a developer, this just looks > like the whole mess should be removed: > > 1) ffmpeg.c , for which the original hack possibly was added in 2012, > now has an option to set output time base as far as I can remember? So > the user of the tool can properly set a specific time base if > required, just like with the AVOption. > 2) This is just extra complexity/automated logic that can be hiding > bugs in API clients, and we ended up having an additional AVOption. > > So, going forward, would patches to remove things like these be worth > sending to the list? yes, but in the more general case it could require a bit of care and could in some cases be alot more work than it initially looks like. It also would likely often require quite a bit of testing and figuring out what the "to be removed" code exactly was needed for. That is testing, code archiology, debuging, and where hacks still are needed design and implementation of better solutions. thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already.
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list firstname.lastname@example.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel