[FFmpeg-devel] [PATCH] Patches to fix issue453 in libdiracschroedinger
Anuradha Suraparaju
anuradha
Wed Jul 9 02:05:30 CEST 2008
Hi,
I see that the patches for issue453 have not been applied yet. Is there
a problem with the patches? Do they need to be changed or corrected?
Regards,
Anuradha
On Mon, 2008-06-16 at 12:58 +1000, Anuradha Suraparaju wrote:
> Hi,
> On Sun, 2008-06-15 at 02:14 +0200, Michael Niedermayer wrote:
> > On Mon, Jun 09, 2008 at 02:26:41PM +1000, Anuradha Suraparaju wrote:
> > > Hi,
> > >
> > >
> > > On Tue, 2008-06-03 at 23:48 +0200, Michael Niedermayer wrote:
> > > > On Sun, Jun 01, 2008 at 06:03:12PM +1000, Anuradha Suraparaju wrote:
> > > > > Hi,
> > > > >
> > > > > I have addressed most of the issues mentioned in your email in the new
> > > > > patches.
> > > > [...]
> > > >
> > > > > > [...]
> > > > > > > Index: libavcodec/dirac_parser.c
> > > > > > > ===================================================================
> > > > > > > --- libavcodec/dirac_parser.c (revision 13233)
> > > > > > > +++ libavcodec/dirac_parser.c (working copy)
> > > > > > > @@ -62,16 +62,12 @@
> > > > > > > ParseContext *pc = s->priv_data;
> > > > > > > int next;
> > > > > > >
> > > > > > > - if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) {
> > > > > > > - next = buf_size;
> > > > > > > - }else{
> > > > > > > - next = find_frame_end(pc, buf, buf_size);
> > > > > > > + next = find_frame_end(pc, buf, buf_size);
> > > > > > >
> > > > > > > - if (ff_combine_frame(pc, next, &buf, &buf_size) < 0) {
> > > > > > > - *poutbuf = NULL;
> > > > > > > - *poutbuf_size = 0;
> > > > > > > - return buf_size;
> > > > > > > - }
> > > > > > > + if (ff_combine_frame(pc, next, &buf, &buf_size) < 0) {
> > > > > > > + *poutbuf = NULL;
> > > > > > > + *poutbuf_size = 0;
> > > > > > > + return buf_size;
> > > > > > > }
> > > > > > >
> > > > > > > *poutbuf = buf;
> > > > > >
> > > > > > The current code in dirac_parser.c looks correct to me, this change should
> > > > > > not be needed.
> > > > > > PARSER_FLAG_COMPLETE_FRAMES is supposed to mean "complete" in the ffmpeg
> > > > > > sense.
> > > > > >
> > > > >
> > > > > As I mentioned in another email, libschroedinger requires that packet
> > > > > sent to it be Dirac byte stream parse units. If a packet contains more
> > > > > than one parse-unit the second gets ignored. Hence the change I made is
> > > > > required.
> > > >
> > > > What ffmpeg calls complete frames is what a parser should output, hence
> > > > if PARSER_FLAG_COMPLETE_FRAMES is set there is no spliting to do for a
> > > > parser.
> > > >
> > > > If i understand the current code correctly the parser does not behave that
> > > > way and the decoder would not work if it did.
> > > > If true -vcodec copy will likely not work with dirac currently
> > > >
> > >
> > > I've left dirac_parser.c unchanged from the svn version and have modfied
> > > libschroedingerdec.c to to sub-parse "complete" ffmpeg frames into
> > > individual parse unit. I am attaching two patches to this email
> > >
> > > issue453_fix_pts_bug_common_libdirac_libschroedinger_svn_13724.diff -
> > > files common to libdirac and libschroedinger
> > > fix_pts_bug_libdirac_svn_13594.diff
> > > issue453_fix_pts_bug_libschroedingerdec_svn_13724.diff
> > >
> > > The earlier patches fix_pts_bug_libdirac_svn_13594.diff (libdiracenc.c)
> > > and issue453_fix_pts_bug_libschroedinger_svn_13594.diff
> > > libschroedingerenc.c) work as is with svn revision 17324.
> >
> > [...]
> >
> >
> > > +static SchroBuffer* FfmpegFindNextSchroParseUnit (FfmpegSchroParseUnitContext *parse_ctx)
> > > +{
> > > + SchroBuffer *enc_buf = NULL;
> > > + int next_pu_offset = 0;
> > > + unsigned char *in_buf;
> > > +
> > > + if (parse_ctx->buf_size < 13 ||
> > > + parse_ctx->buf[0] != 'B' ||
> > > + parse_ctx->buf[1] != 'B' ||
> > > + parse_ctx->buf[2] != 'C' ||
> > > + parse_ctx->buf[3] != 'D')
> > > + return NULL;
> > > +
> > > + next_pu_offset = (parse_ctx->buf[5] << 24) +
> > > + (parse_ctx->buf[6] << 16) +
> > > + (parse_ctx->buf[7] << 8) +
> > > + parse_ctx->buf[8];
> > > +
> > > + if (next_pu_offset == 0 &&
> > > + SCHRO_PARSE_CODE_IS_END_OF_SEQUENCE(parse_ctx->buf[4]))
> > > + next_pu_offset = 13;
> > > +
> > > + if (parse_ctx->buf_size < next_pu_offset)
> > > + return NULL;
> >
> > both buf_size and next_pu_offset are signed and it seems next_pu_offset could
> > easily be <0 given the wrong input. If that happened this condition would be
> > false even though it surely should be true.
>
> Fixed. Changed this condition to
> if ( next_pu_offset <= 0 || parse_ctx->buf_size < next_pu_offset)
> return NULL;
>
> A null offset for a non-End of Sequence packet is also not valid.
> >
> >
> > > +
> > > + in_buf = av_malloc(next_pu_offset);
> > > + memcpy (in_buf, parse_ctx->buf, next_pu_offset);
> > > + enc_buf = schro_buffer_new_with_data (in_buf, next_pu_offset);
> > > + enc_buf->free = libschroedinger_decode_buffer_free;
> > > + enc_buf->priv = in_buf;
> > > +
> > > + parse_ctx->buf += next_pu_offset;
> > > + parse_ctx->buf_size -= next_pu_offset;
> > > +
> > > + return enc_buf;
> > > +}
> > > +
> > > /**
> > > * Returns FFmpeg chroma format.
> > > */
> > > @@ -164,30 +218,33 @@
> > > SchroFrame* frame;
> > > int state;
> > > int go = 1;
> > > + int outer = 1;
> > > + FfmpegSchroParseUnitContext parse_ctx;
> > >
> > > *data_size = 0;
> > >
> > > - if (buf_size>0) {
> > > - unsigned char *in_buf = av_malloc(buf_size);
> > > - memcpy (in_buf, buf, buf_size);
> > > - enc_buf = schro_buffer_new_with_data (in_buf, buf_size);
> > > - enc_buf->free = libschroedinger_decode_buffer_free;
> > > - enc_buf->priv = in_buf;
> > > - /* Push buffer into decoder. */
> > > - state = schro_decoder_push (decoder, enc_buf);
> > > - if (state == SCHRO_DECODER_FIRST_ACCESS_UNIT)
> > > - libschroedinger_handle_first_access_unit(avccontext);
> > > - } else {
> > > + FfmpegSchroParseContextInit (&parse_ctx, buf, buf_size);
> > > + if (buf_size == 0) {
> >
> > > - if (!p_schro_params->eos_signalled) {
> > > + if (!p_schro_params->eos_signalled) {
> > > - state = schro_decoder_push_end_of_stream(decoder);
> > > + state = schro_decoder_push_end_of_stream(decoder);
> > > - p_schro_params->eos_signalled = 1;
> > > + p_schro_params->eos_signalled = 1;
> > > - }
> > > + }
> >
> > cosmetics, and the indention is even less correct afterwards.
>
> Fixed.
>
> >
> >
> > > }
> > >
> > > + /* Loop through all the individual parse units in the input buffer */
> > > + do {
> > > + if ((enc_buf = FfmpegFindNextSchroParseUnit(&parse_ctx))) {
> > > + /* Push buffer into decoder. */
> > > + state = schro_decoder_push (decoder, enc_buf);
> > > + if (state == SCHRO_DECODER_FIRST_ACCESS_UNIT)
> > > + libschroedinger_handle_first_access_unit(avccontext);
> > > + go = 1;
> > > + }
> > > + else
> > > + outer = 0;
> >
> > > + format = p_schro_params->format;
> > > - format = p_schro_params->format;
> > > -
> > > - while (go) {
> > > /* Parse data and process result. */
> > > + while (go) {
> >
> > cosmetics
>
> Fixed.
>
> >
> > and the other patch in this email looks ok
> >
> > [...]
>
> Regards,
> Anuradha
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list