On Tue, Apr 28, 2015 at 07:27:06PM +0200, Michael Niedermayer wrote:
> On Tue, Apr 28, 2015 at 05:45:35PM +0300, foo86 wrote:
> > Check extended sync word for 16-bit LE and BE core streams to reduce
> > probability of alias sync detection. Previously sync word extension was
> > checked only for 14-bit streams.
> >
> > This is sufficient to make the sample in ticket #4492 parse successfully.
> > The proper solution, however, would involve taking extension sub-stream
> > length into account by the parser.
> > ---
> > libavcodec/dca_parser.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/dca_parser.c b/libavcodec/dca_parser.c
> > index 9797760..41658a5 100644
> > --- a/libavcodec/dca_parser.c
> > +++ b/libavcodec/dca_parser.c
> > @@ -38,7 +38,9 @@ typedef struct DCAParseContext {
> > #define IS_MARKER(state, i, buf, buf_size) \
> > ((state == DCA_SYNCWORD_CORE_14B_LE && (i < buf_size - 2) && (buf[i +
> > 1] & 0xF0) == 0xF0 && buf[i + 2] == 0x07) || \
> > (state == DCA_SYNCWORD_CORE_14B_BE && (i < buf_size - 2) && buf[i +
> > 1] == 0x07 && (buf[i + 2] & 0xF0) == 0xF0) || \
> > - state == DCA_SYNCWORD_CORE_LE || state == DCA_SYNCWORD_CORE_BE ||
> > state == DCA_SYNCWORD_SUBSTREAM)
> > + (state == DCA_SYNCWORD_CORE_LE && (i < buf_size - 2) && (buf[i +
> > 2] & 0xFC) == 0xFC) || \
> > + (state == DCA_SYNCWORD_CORE_BE && (i < buf_size - 1) && (buf[i +
> > 1] & 0xFC) == 0xFC) || \
> > + state == DCA_SYNCWORD_SUBSTREAM)
>
> i dont think this is safe, buf_size could be always 1, a parser
> could be feeded by 1 byte only in each call
You are right. I didn't realize existing check was unsafe before reusing
it.
> also testing with a few random dts files this patch causes many
> changes like:
> is this intended ?
>
> tested with https://samples.mplayerhq.hu/A-codecs/DTS/dts/5.1%2024bit.dts
>
> --- ref2.crc 2015-04-28 19:21:47.619285678 +0200
> +++ new2.crc 2015-04-28 19:21:44.187285606 +0200
> @@ -56,8 +56,7 @@
> , 960, 2012, 0x2583fa70
> , 960, 2012, 0x979ff3f3
> , 960, 2012, 0xbc32f58f
> -, 960, 2012, 0xbef8eb01
> -, 960, 2012, 0x9a37115b
> +, 960, 4024, 0xc153fc5c
> , 960, 2012, 0x5625f402
> , 960, 2012, 0xab29f377
> , 960, 2012, 0xc528e7ad
> @@ -312,8 +311,7 @@
> , 960, 2012, 0x334da9f1
> , 960, 2012, 0xbd47b990
> , 960, 2012, 0x4fa0cd82
> -, 960, 2012, 0x3bc2c8cb
> -, 960, 2012, 0xa68ccb2b
> +, 960, 4024, 0x5a479405
> , 960, 2012, 0x3c64bf12
> , 960, 2012, 0xb473bd06
> , 960, 2012, 0x9b1ac192
>
> [...]
This is not intended, likely the consequence of IS_MARKER check not
working correctly at the buffer boundary. I'll try to come up with a
better way to fix sync detection.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
