On Sun, 5 Apr 2015, Derek Buitenhuis wrote:

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

Well, it simply can't be the same, given the way it currently is implemented (when it is implemented as a number of TS packets - for strict CBR, a specific time duration can easily be calculated back to a number of packets, while it really can be just about anything for VBR).

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.

Probably because mpegts really is CBR and the VBR case is a hack, and impelementing it as a fixed count of packets is simple and does make sense there.

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

What about keeping the option in millisecond units, and calculate an average (and quite approximate) muxrate as a sum of all the streams individual bitrates, and calculate a packet interval based on that. It won't be exact and fully correct, but it'd be a sensible unit for setting it in, it'd make sense for CBR. (And for reducing the rate of outputting these packets, you can still easily set it to over 9000, regardless of unit.)

What's left is what to do for VBR, when there's no bitrate set for the individual streams (which can happen with stream copy if the demuxer doesn't set the bitrate). We could fall back to some other default, i.e. assume total muxrate of 1 mbps or so, and calculate a packet interval based on the ms value. It'd be completely arbitrary just as the value right now, but at least settable somehow.

// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to