| Message ID | 20180411114040.1596-1-h.leppkes@gmail.com |
|---|---|
| State | New |
| Headers | show |
2018-04-11 13:40 GMT+02:00, Hendrik Leppkes <h.leppkes@gmail.com>: > If a frame starts very close to a packet boundary, the start code may > already have been added to the parsing buffer, indicated by a small > negative value of "i", while the header is still being tracked in the > "state" variable. Do you have a sample? Carl Eugen
On Wed, Apr 11, 2018 at 8:07 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2018-04-11 13:40 GMT+02:00, Hendrik Leppkes <h.leppkes@gmail.com>: >> If a frame starts very close to a packet boundary, the start code may >> already have been added to the parsing buffer, indicated by a small >> negative value of "i", while the header is still being tracked in the >> "state" variable. > > Do you have a sample? > I do, but unless you can tell me how you can get ffmpeg to split packets from a DVD VOB stream on a fixed arbitrary packet size (2048 bytes, iirc) to cause the issue to appear, I wouldn't know how to reproduce it from CLI. - Hendrik
On Wed, Apr 11, 2018 at 1:40 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > If a frame starts very close to a packet boundary, the start code may > already have been added to the parsing buffer, indicated by a small > negative value of "i", while the header is still being tracked in the > "state" variable. > > Reduce the remaining size accordingly, otherwise trying to find the next > frame could skip over the frame header and lump two frames together as > one. > --- > libavcodec/aac_ac3_parser.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c > index 019074b0dd..7f20626285 100644 > --- a/libavcodec/aac_ac3_parser.c > +++ b/libavcodec/aac_ac3_parser.c > @@ -60,6 +60,8 @@ get_next: > s->remaining_size += i; > goto get_next; > } > + else if (i < 0) > + s->remaining_size += i; > } > } > } > -- > 2.16.1.windows.4 > Ping. Note that this is somewhat of a regression in AC3 parsing since the recent AC3+EAC3 combined stream support (the bug existed before, but that change exposed it). - Hendrik
On 4/13/18, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > On Wed, Apr 11, 2018 at 1:40 PM, Hendrik Leppkes <h.leppkes@gmail.com> > wrote: >> If a frame starts very close to a packet boundary, the start code may >> already have been added to the parsing buffer, indicated by a small >> negative value of "i", while the header is still being tracked in the >> "state" variable. >> >> Reduce the remaining size accordingly, otherwise trying to find the next >> frame could skip over the frame header and lump two frames together as >> one. >> --- >> libavcodec/aac_ac3_parser.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c >> index 019074b0dd..7f20626285 100644 >> --- a/libavcodec/aac_ac3_parser.c >> +++ b/libavcodec/aac_ac3_parser.c >> @@ -60,6 +60,8 @@ get_next: >> s->remaining_size += i; >> goto get_next; >> } >> + else if (i < 0) >> + s->remaining_size += i; >> } >> } >> } >> -- >> 2.16.1.windows.4 >> > > Ping. > Note that this is somewhat of a regression in AC3 parsing since the > recent AC3+EAC3 combined stream support (the bug existed before, but > that change exposed it). Please add { } brackets around new code. Patch LGTM otherwise.
On Fri, Apr 13, 2018 at 11:54 AM, Paul B Mahol <onemda@gmail.com> wrote: > On 4/13/18, Hendrik Leppkes <h.leppkes@gmail.com> wrote: >> On Wed, Apr 11, 2018 at 1:40 PM, Hendrik Leppkes <h.leppkes@gmail.com> >> wrote: >>> If a frame starts very close to a packet boundary, the start code may >>> already have been added to the parsing buffer, indicated by a small >>> negative value of "i", while the header is still being tracked in the >>> "state" variable. >>> >>> Reduce the remaining size accordingly, otherwise trying to find the next >>> frame could skip over the frame header and lump two frames together as >>> one. >>> --- >>> libavcodec/aac_ac3_parser.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c >>> index 019074b0dd..7f20626285 100644 >>> --- a/libavcodec/aac_ac3_parser.c >>> +++ b/libavcodec/aac_ac3_parser.c >>> @@ -60,6 +60,8 @@ get_next: >>> s->remaining_size += i; >>> goto get_next; >>> } >>> + else if (i < 0) >>> + s->remaining_size += i; >>> } >>> } >>> } >>> -- >>> 2.16.1.windows.4 >>> >> >> Ping. >> Note that this is somewhat of a regression in AC3 parsing since the >> recent AC3+EAC3 combined stream support (the bug existed before, but >> that change exposed it). > > Please add { } brackets around new code. > Applied with that changed. - Hendrik
diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c index 019074b0dd..7f20626285 100644 --- a/libavcodec/aac_ac3_parser.c +++ b/libavcodec/aac_ac3_parser.c @@ -60,6 +60,8 @@ get_next: s->remaining_size += i; goto get_next; } + else if (i < 0) + s->remaining_size += i; } } }