[FFmpeg-devel] [PATCH v3 3/6] avformat/s337m: Make available as subdemuxer

[Gaullier Nicolas]

>> +        if (pos < size - 9 && pos >= S337M_PROBE_GUARDBAND_MIN_BYTES) 
>I think this 9 should be an 11 or 12..
Indeed, thank you, my mistake.

>This isn't quite what I meant by turning it into an integer test :) this will likely just be rounded to zero. I meant that things could likely be rearranged so there's no divisions, so the probing isn't subject to the vaguaries of float >rounding
On one side, dealing with a macro that contains a double (S337M_PHASE_PROBE_MIN) makes it impossible at the end to avoid a test involving float rounding (in my understanding, but I may be wrong, and I remember there are some helpers in ffmpeg to make it easier to avoid float rounding), or maybe I should have limited the digits and simply worked with an integer number of milliseconds, but that sounded a little bit overkill, together with the fact that the default value is still 0.0000 so all of this is only in case a developer wants to change the #define.
On the other side, I don't think it was such a good idea to specify this in seconds as for the dolby_e min/max. This value is just for assurance that there will be no wrong probing, and I thought that just byte count was more appropriate... and that makes it an integer test at the end :)
So I replaced : 
  if (pos < size - 9 && 
  (s337m_phase = (double)pos * 4 / (*st)->codecpar->bits_per_coded_sample / (*st)->codecpar->sample_rate) >= S337M_PHASE_PROBE_MIN) {
With just simply :
  if (pos >= S337M_PROBE_GUARDBAND_MIN_BYTES)
And thus replaced S337M_PHASE_PROBE_MIN=0.000 by S337M_PROBE_GUARDBAND_MIN_BYTES=0.
At the end, I don't see any cons to do that, and it is far most simplest.

> +#define DOLBY_E_PHASE_MIN       0.000610
> +#define DOLBY_E_PHASE_MAX       0.001050
>Where do these phase values come from? Is there a spec somewhere?
https://www.dolby.com/us/en/technologies/dolby-e-line-position.pdf

Nicolas