| Message ID | 20210411135601.611185-1-Shulyaka@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] Fix SPDIF detection score | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Hi! How do I ask for a review? I am new here. вс, 11 апр. 2021 г. в 16:56, Denis Shulyaka <shulyaka@gmail.com>: > This patch fixes the detection score for spdif (IEC61937). > > Signed-off-by: Denis Shulyaka <Shulyaka@gmail.com> > --- > libavformat/spdifdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/spdifdec.c b/libavformat/spdifdec.c > index 1808fa9d65..05c155f49c 100644 > --- a/libavformat/spdifdec.c > +++ b/libavformat/spdifdec.c > @@ -140,7 +140,7 @@ int ff_spdif_probe(const uint8_t *p_buf, int buf_size, > enum AVCodecID *codec) > break; > > /* continue probing to find more sync codes */ > - probe_end = FFMIN(buf + SPDIF_MAX_OFFSET, p_buf + buf_size - > 1); > + probe_end = FFMIN(buf + SPDIF_MAX_OFFSET + 1, p_buf + > buf_size - 1); > > /* skip directly to the next sync code */ > if (!spdif_get_offset_and_codec(NULL, (buf[2] << 8) | buf[1], > -- > 2.30.2 > >
Am So., 11. Apr. 2021 um 15:56 Uhr schrieb Denis Shulyaka <shulyaka@gmail.com>: > > This patch fixes the detection score for spdif (IEC61937). > > Signed-off-by: Denis Shulyaka <Shulyaka@gmail.com> > --- > libavformat/spdifdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/spdifdec.c b/libavformat/spdifdec.c > index 1808fa9d65..05c155f49c 100644 > --- a/libavformat/spdifdec.c > +++ b/libavformat/spdifdec.c > @@ -140,7 +140,7 @@ int ff_spdif_probe(const uint8_t *p_buf, int buf_size, enum AVCodecID *codec) > break; > > /* continue probing to find more sync codes */ > - probe_end = FFMIN(buf + SPDIF_MAX_OFFSET, p_buf + buf_size - 1); > + probe_end = FFMIN(buf + SPDIF_MAX_OFFSET + 1, p_buf + buf_size - 1); Do you have a sample that shows that this patch fixes detection? Carl Eugen
On Tue, Jun 15, 2021 at 12:52 PM Denis Shulyaka <shulyaka@gmail.com> wrote: > > Hi! How do I ask for a review? I am new here. > A short explanation of what the problem is and why this change fixes it would help us to understand and evaluate the patch - and generally makes for better patches, as any future reader will be able to look up where the +1 came from and why it should be there. :) - Hendrik
Sorry for not giving the proper description! Here it is: The spdif is based on blocks with fixed length, each block starts with a sync word. It matters how many sync words we find during probe and whether we find them in expected place. The SPDIF_MAX_OFFSET constant is the maximum supported block side. When we find a sync word, we shift the upper boundary of the search area (probe_end) by SPDIF_MAX_OFFSET, however this boundary is exclusive (the condition in the loop is "strictly less"), and when the actual block size matches the max supported block size, then the second sync word appears to be outside of the search area by 1 byte. As a result, we find only one sync word and return an incorrect score (AVPROBE_SCORE_EXTENSION / 4 instead of AVPROBE_SCORE_MAX). The fix gives 1 more byte to the search area to allow finding the subsequent sync word. To reproduce the issue you would need a sample with SPDIF_MAX_OFFSET (16384) block size (probably AAC with samples = 4096). I do not have such sample as I found this issue while working on a new codec support. ср, 16 июн. 2021 г., 01:18 Hendrik Leppkes <h.leppkes@gmail.com>: > On Tue, Jun 15, 2021 at 12:52 PM Denis Shulyaka <shulyaka@gmail.com> > wrote: > > > > Hi! How do I ask for a review? I am new here. > > > > A short explanation of what the problem is and why this change fixes > it would help us to understand and evaluate the patch - and generally > makes for better patches, as any future reader will be able to look up > where the +1 came from and why it should be there. :) > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Hi all, This is a kind reminder. Best regards, Denis Shulyaka ср, 16 июн. 2021 г. в 02:07, Denis Shulyaka <shulyaka@gmail.com>: > Sorry for not giving the proper description! Here it is: > The spdif is based on blocks with fixed length, each block starts with a > sync word. It matters how many sync words we find during probe and whether > we find them in expected place. The SPDIF_MAX_OFFSET constant is the > maximum supported block side. When we find a sync word, we shift the upper > boundary of the search area (probe_end) by SPDIF_MAX_OFFSET, however this > boundary is exclusive (the condition in the loop is "strictly less"), and > when the actual block size matches the max supported block size, then the > second sync word appears to be outside of the search area by 1 byte. As a > result, we find only one sync word and return an incorrect score > (AVPROBE_SCORE_EXTENSION / 4 instead of AVPROBE_SCORE_MAX). The fix gives 1 > more byte to the search area to allow finding the subsequent sync word. > To reproduce the issue you would need a sample with SPDIF_MAX_OFFSET > (16384) block size (probably AAC with samples = 4096). I do not have such > sample as I found this issue while working on a new codec support. > > ср, 16 июн. 2021 г., 01:18 Hendrik Leppkes <h.leppkes@gmail.com>: > >> On Tue, Jun 15, 2021 at 12:52 PM Denis Shulyaka <shulyaka@gmail.com> >> wrote: >> > >> > Hi! How do I ask for a review? I am new here. >> > >> >> A short explanation of what the problem is and why this change fixes >> it would help us to understand and evaluate the patch - and generally >> makes for better patches, as any future reader will be able to look up >> where the +1 came from and why it should be there. :) >> >> - Hendrik >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> >
diff --git a/libavformat/spdifdec.c b/libavformat/spdifdec.c index 1808fa9d65..05c155f49c 100644 --- a/libavformat/spdifdec.c +++ b/libavformat/spdifdec.c @@ -140,7 +140,7 @@ int ff_spdif_probe(const uint8_t *p_buf, int buf_size, enum AVCodecID *codec) break; /* continue probing to find more sync codes */ - probe_end = FFMIN(buf + SPDIF_MAX_OFFSET, p_buf + buf_size - 1); + probe_end = FFMIN(buf + SPDIF_MAX_OFFSET + 1, p_buf + buf_size - 1); /* skip directly to the next sync code */ if (!spdif_get_offset_and_codec(NULL, (buf[2] << 8) | buf[1],
This patch fixes the detection score for spdif (IEC61937). Signed-off-by: Denis Shulyaka <Shulyaka@gmail.com> --- libavformat/spdifdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)