On 4/5/2015 8:41 PM, Martin Storsjö wrote: > This changes the default value for the CBR case
I know. There was no explanation why it differed. (I also tend to think separate defaults are just insane.) We should keep one or the other, but not both. > This is a bit problematic, when one single option is interpreted in two > different units depending on the case. For the CBR case, SDT_RETRANS_TIME > previously (and ts->sdt_period after this patch) is interpreted as a value > in milliseconds, while for the VBR case below it is interpreted as a > number of packets. Someone ought to go back in time and ask Fabrice why. (Before someone chimes in with a "while you're at it": No, I will not refactor mpegtsenc.c to be consistent, just for this patch set to be accepted > I'm a bit unsure how to expose this in the best way via an avoption > actually - two different avoptions for the two cases is ugly, while one > single option interpreted in two different ways also is problematic. Also, > if changing the default value for any case, I'd rather change the value > for the VBR case (the CBR case default value makes more sense I think). Well, being able to set these values for the CBR case is arguably even more important (broadcast standards, etc.) I am fine with changing the VBR case defaults. As for how to best expose it, one solution is simply to to document it properly. This would be my preference, rather than adding a total of four new avoptions. Any other preferences? - Derek _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
