[FFmpeg-devel] [PATCH 3/3] doc/muxers: Document range for mpegts periods

[Ganesh Ajjanagadde]

On Sat, Oct 17, 2015 at 1:49 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Sat, Oct 17, 2015 at 12:43:32PM -0400, Ganesh Ajjanagadde wrote:
>> On Sat, Oct 17, 2015 at 12:33 PM, Michael Niedermayer
>> <michael at niedermayer.cc> wrote:
>> > On Fri, Oct 16, 2015 at 03:09:21PM -0400, Derek Buitenhuis wrote:
>> >> Signed-off-by: Derek Buitenhuis <derek.buitenhuis at gmail.com>
>> >> ---
>> >>  doc/muxers.texi | 4 ++--
>> >>  1 file changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/doc/muxers.texi b/doc/muxers.texi
>> >> index 06483fa..cef04e1 100644
>> >> --- a/doc/muxers.texi
>> >> +++ b/doc/muxers.texi
>> >> @@ -802,9 +802,9 @@ Set a constant muxrate (default VBR).
>> >>  Override the default PCR retransmission time (default 20ms), ignored
>> >>  if variable muxrate is selected.
>> >>  @item pat_period @var{number}
>> >> -Maximal time in seconds between PAT/PMT tables.
>> >> +Maximal time in seconds between PAT/PMT tables. Max value is INT_MAX / 2 - 1.
>> >>  @item sdt_period @var{number}
>> >> -Maximal time in seconds between SDT tables.
>> >> +Maximal time in seconds between SDT tables. Max value is INT_MAX / 2 - 1.
>> >
>> > iam unsure about this, INT_MAX is theoretically platform specific and
>> > most non developers probably dont know its value
>> > that said, iam not against it at all, just feels like this cant be
>> > the best solution
>>
>> Certainly not the best, but definitely better than the current - range
>> restrictions should be documented. Furthermore, INT_MAX is pretty well
>> known even for a first exposure to C, and in all likelihood someone
>> who reads the muxer docs is sharp/knows enough to do a quick google on
>> what INT_MAX is (after all, he was savvy enough to look at the docs).
>>
>> Maybe one could in principle use configure to get the relevant actual
>> value and use that in the docs. That is pretty complex and should be
>> done IMHO only if there are more such type-width specific limits.
>> Furthermore, that has problems, if docs were obtained off a website
>> (website's config might be different from user's, etc). Given this, I
>> still favor the INT_MAX solution.
>
> theres mayybe no need to document the exact maximum, stating that 2^30
> is the max could be fine (its 34 years) or the code could be changed as
> well to not have "INT_MAX / 2 - 1" as maximum
> theres also the aspect that if int is 64bit, INT_MAX / 2 - 1 cannot
> exactly be represented as IEEE 64bit double, i doubt that matters but
> this patchset seemed to have the goal to get this precissely correct

There are a number of such things, but I think they belong to a separate patch.

>
>
>>
>> Also, on that note, it has already been noted that on all platforms we
>> have (currently) int is 32 bits. A lot more serious things will break
>> than the docs if the width of ints changes. That is another thing in
>> INT_MAX's favor: it won't break on such a change :).
>
> what breaks with 64bit int ?

Look at ff_ctz_c (before or after the De-Bruijn commit) - it does not
compute the right thing if int is 64 bits. Whether this causes
"breakage" I can't say, but the code is definitely incorrect. I am
sure there are plenty of other such instances, and out of those it is
likely one of them actually causes breakage to some CLI usage, API
usage, etc.

> (note if you say x86 asm then the question is which x86 ABI allows 64
>  bit int, if thats none then non x86 is implied by 64bit int)
>
> also is there a way to (easily) test a build with 64bit int ?

I don't know, but I am eager to know this as well. In particular, I
would be curious to test my hypothesis above.

>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If you think the mosad wants you dead since a long time then you are either
> wrong or dead since a long time.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>