diff mbox

[FFmpeg-devel,v2,5/6] avcodec/cbs_jpeg: Use memcpy when writing pictures

Message ID 20191117073440.22718-6-andreas.rheinhardt@gmail.com
State Accepted
Headers show

Commit Message

Andreas Rheinhardt Nov. 17, 2019, 7:34 a.m. UTC
This is possible because the size of a scan header is always a multiple
of a byte.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/cbs_jpeg.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Mark Thompson Nov. 18, 2019, 12:02 a.m. UTC | #1
On 17/11/2019 07:34, Andreas Rheinhardt wrote:
> This is possible because the size of a scan header is always a multiple
> of a byte.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/cbs_jpeg.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
> index b189cbd9b7..64fe70beab 100644
> --- a/libavcodec/cbs_jpeg.c
> +++ b/libavcodec/cbs_jpeg.c
> @@ -330,7 +330,7 @@ static int cbs_jpeg_write_scan(CodedBitstreamContext *ctx,
>                                 PutBitContext *pbc)
>  {
>      JPEGRawScan *scan = unit->content;
> -    int i, err;
> +    int err;
>  
>      err = cbs_jpeg_write_scan_header(ctx, pbc, &scan->header);
>      if (err < 0)
> @@ -340,8 +340,12 @@ static int cbs_jpeg_write_scan(CodedBitstreamContext *ctx,
>          if (scan->data_size * 8 > put_bits_left(pbc))
>              return AVERROR(ENOSPC);
>  
> -        for (i = 0; i < scan->data_size; i++)
> -            put_bits(pbc, 8, scan->data[i]);
> +        av_assert0(put_bits_count(pbc) % 8 == 0);
> +
> +        flush_put_bits(pbc);

Doesn't this pad to a word boundary?  (It looks like that's coincidentally fine for a 3-component JPEG, but not in general.)

> +
> +        memcpy(put_bits_ptr(pbc), scan->data, scan->data_size);
> +        skip_put_bytes(pbc, scan->data_size);
>      }
>  
>      return 0;
> 

- Mark
Andreas Rheinhardt Nov. 18, 2019, 2:54 a.m. UTC | #2
On Mon, Nov 18, 2019 at 1:02 AM Mark Thompson <sw@jkqxz.net> wrote:

> On 17/11/2019 07:34, Andreas Rheinhardt wrote:
> > This is possible because the size of a scan header is always a multiple
> > of a byte.
> >
> > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> > ---
> >  libavcodec/cbs_jpeg.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
> > index b189cbd9b7..64fe70beab 100644
> > --- a/libavcodec/cbs_jpeg.c
> > +++ b/libavcodec/cbs_jpeg.c
> > @@ -330,7 +330,7 @@ static int cbs_jpeg_write_scan(CodedBitstreamContext
> *ctx,
> >                                 PutBitContext *pbc)
> >  {
> >      JPEGRawScan *scan = unit->content;
> > -    int i, err;
> > +    int err;
> >
> >      err = cbs_jpeg_write_scan_header(ctx, pbc, &scan->header);
> >      if (err < 0)
> > @@ -340,8 +340,12 @@ static int
> cbs_jpeg_write_scan(CodedBitstreamContext *ctx,
> >          if (scan->data_size * 8 > put_bits_left(pbc))
> >              return AVERROR(ENOSPC);
> >
> > -        for (i = 0; i < scan->data_size; i++)
> > -            put_bits(pbc, 8, scan->data[i]);
> > +        av_assert0(put_bits_count(pbc) % 8 == 0);
> > +
> > +        flush_put_bits(pbc);
>
> Doesn't this pad to a word boundary?  (It looks like that's coincidentally
> fine for a 3-component JPEG, but not in general.)
>
>
No, it doesn't. It just writes everything in its cache, filling any unset
bits in the last byte with zero. And it does so by writing single bytes at
a time.

- Andreas
diff mbox

Patch

diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
index b189cbd9b7..64fe70beab 100644
--- a/libavcodec/cbs_jpeg.c
+++ b/libavcodec/cbs_jpeg.c
@@ -330,7 +330,7 @@  static int cbs_jpeg_write_scan(CodedBitstreamContext *ctx,
                                PutBitContext *pbc)
 {
     JPEGRawScan *scan = unit->content;
-    int i, err;
+    int err;
 
     err = cbs_jpeg_write_scan_header(ctx, pbc, &scan->header);
     if (err < 0)
@@ -340,8 +340,12 @@  static int cbs_jpeg_write_scan(CodedBitstreamContext *ctx,
         if (scan->data_size * 8 > put_bits_left(pbc))
             return AVERROR(ENOSPC);
 
-        for (i = 0; i < scan->data_size; i++)
-            put_bits(pbc, 8, scan->data[i]);
+        av_assert0(put_bits_count(pbc) % 8 == 0);
+
+        flush_put_bits(pbc);
+
+        memcpy(put_bits_ptr(pbc), scan->data, scan->data_size);
+        skip_put_bytes(pbc, scan->data_size);
     }
 
     return 0;