[FFmpeg-devel] [PATCH]lavc/mpegvideo_enc: Do not write an extra RST tag after final jpeg slice

[Michael Niedermayer]

On Mon, Jan 20, 2020 at 11:59:18PM +0100, Carl Eugen Hoyos wrote:
> Am Mo., 20. Jan. 2020 um 23:34 Uhr schrieb Michael Niedermayer
> <michael at niedermayer.cc>:
> >
> > On Mon, Jan 20, 2020 at 11:15:31PM +0100, Carl Eugen Hoyos wrote:
> > > Am Mo., 20. Jan. 2020 um 22:51 Uhr schrieb Michael Niedermayer
> > > <michael at niedermayer.cc>:
> > > >
> > > > On Mon, Jan 20, 2020 at 12:54:21AM +0100, Carl Eugen Hoyos wrote:
> > > > > Hi!
> > > > >
> > > > > Attached patch fixes ticket #8412.
> > > > >
> > > > > Please comment, Carl Eugen
> > > >
> > > > >  mpegvideo_enc.c |    7 ++++---
> > > > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > > > 73d8636d0dba4f02cf1697b622c97945d738f06f  0001-lavc-mpegvideo_enc-Do-not-write-an-extra-RST-tag-aft.patch
> > > > > From 7f62ffa46f9264c1fc7854def8467816e0173883 Mon Sep 17 00:00:00 2001
> > > > > From: Carl Eugen Hoyos <ceffmpeg at gmail.com>
> > > > > Date: Mon, 20 Jan 2020 00:51:33 +0100
> > > > > Subject: [PATCH] lavc/mpegvideo_enc: Do not write an extra RST tag after final
> > > > >  jpeg slice.
> > > > >
> > > > > Fixes ticket #8412.
> > > > > ---
> > > > >  libavcodec/mpegvideo_enc.c | 7 ++++---
> > > > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> > > > > index 96f5b4a666..fc1c46fee7 100644
> > > > > --- a/libavcodec/mpegvideo_enc.c
> > > > > +++ b/libavcodec/mpegvideo_enc.c
> > > > > @@ -3566,8 +3566,9 @@ static void merge_context_after_me(MpegEncContext *dst, MpegEncContext *src){
> > > > >      MERGE(me.mb_var_sum_temp);
> > > > >  }
> > > > >
> > > > > -static void merge_context_after_encode(MpegEncContext *dst, MpegEncContext *src){
> > > > > +static void merge_context_after_encode(MpegEncContext *dst, MpegEncContext *src, int last){
> > > > >      int i;
> > > > > +    int put_bits = put_bits_count(&src->pb) - (src->out_format == FMT_MJPEG && src->avctx->active_thread_type & FF_THREAD_SLICE && last ? 32 : 0);
> > > > >
> > > > >      MERGE(dct_count[0]); //note, the other dct vars are not part of the context
> > > > >      MERGE(dct_count[1]);
> > > > > @@ -3594,7 +3595,7 @@ static void merge_context_after_encode(MpegEncContext *dst, MpegEncContext *src)
> > > > >
> > > > >      av_assert1(put_bits_count(&src->pb) % 8 ==0);
> > > > >      av_assert1(put_bits_count(&dst->pb) % 8 ==0);
> > > > > -    avpriv_copy_bits(&dst->pb, src->pb.buf, put_bits_count(&src->pb));
> > > > > +    avpriv_copy_bits(&dst->pb, src->pb.buf, put_bits);
> > > > >      flush_put_bits(&dst->pb);
> > > > >  }
> > > > >
> > > > > @@ -3919,7 +3920,7 @@ static int encode_picture(MpegEncContext *s, int picture_number)
> > > > >      for(i=1; i<context_count; i++){
> > > > >          if (s->pb.buf_end == s->thread_context[i]->pb.buf)
> > > > >              set_put_bits_buffer_size(&s->pb, FFMIN(s->thread_context[i]->pb.buf_end - s->pb.buf, INT_MAX/8-32));
> > > > > -        merge_context_after_encode(s, s->thread_context[i]);
> > > > > +        merge_context_after_encode(s, s->thread_context[i], i == context_count - 1);
> > > > >      }
> > > > >      emms_c();
> > > > >      return 0;
> > > > > --
> > > > > 2.23.0
> > > >
> > > > this looks a bit unexpected
> > > > is there a reason why the code that writes the RST is not skipped ?
> > >
> > > Sorry if I misunderstand but how would one thread know that it is
> > > the last one and no RST should be written?
> >
> > the thread encoding the last (as in bottom right) MB should be the last
> > slice thread
> 
> New patch attached, I failed to understand the code yesterday.
> 
> Please comment, Carl Eugen

>  mjpegenc_common.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 5c76a5298352eb275764526ebe2d7312c98d1c26  0001-lavc-mjpegenc-Fix-not-writing-RST-tag-after-final-sl.patch
> From f50caafa55f54a4987a73cccb2b954e3e137781e Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceffmpeg at gmail.com>
> Date: Mon, 20 Jan 2020 23:57:38 +0100
> Subject: [PATCH] lavc/mjpegenc: Fix not writing RST tag after final slice.
> 
> Fixes ticket #8412.
> ---
>  libavcodec/mjpegenc_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
> index 31868c9bed..3038ebde6e 100644
> --- a/libavcodec/mjpegenc_common.c
> +++ b/libavcodec/mjpegenc_common.c
> @@ -573,7 +573,7 @@ int ff_mjpeg_encode_stuffing(MpegEncContext *s)
>  
>      ff_mjpeg_escape_FF(pbc, s->esc_pos);
>  
> -    if((s->avctx->active_thread_type & FF_THREAD_SLICE) && mb_y < s->mb_height)
> +    if((s->avctx->active_thread_type & FF_THREAD_SLICE) && mb_y < s->mb_height - 1)
>          put_marker(pbc, RST0 + (mb_y&7));
>      s->esc_pos = put_bits_count(pbc) >> 3;
>  fail:

LGTM

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200121/b81c25eb/attachment.sig>