[01/13] rtpdec_jpeg: Write the DHT section properly

Message ID 1347363979-4676-1-git-send-email-martin@martin.st
State Committed
Commit 7ef4323405fae8f62732111b747d585ef9c786c7
Headers show

Commit Message

Martin Storsjö Sept. 11, 2012, 11:46 a.m.
Currently the size header of the generated DHT section is
incorrect, making the mjpeg decoder just skip it. Since the
written huffman tables are the default ones, this failure had
gone undetected.
---
 libavformat/rtpdec_jpeg.c |   33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

Comments

Luca Barbato Sept. 11, 2012, 1:35 p.m. | #1
On 9/11/12 1:46 PM, Martin Storsjö wrote:
> Currently the size header of the generated DHT section is
> incorrect, making the mjpeg decoder just skip it. Since the
> written huffman tables are the default ones, this failure had
> gone undetected.
> ---
>   libavformat/rtpdec_jpeg.c |   33 ++++++++++++++++++++-------------
>   1 file changed, 20 insertions(+), 13 deletions(-)


Looks fine
Samuel Pitoiset Sept. 11, 2012, 2:46 p.m. | #2
On Tue, Sep 11, 2012 at 1:46 PM, Martin Storsjö <martin@martin.st> wrote:
> Currently the size header of the generated DHT section is
> incorrect, making the mjpeg decoder just skip it. Since the
> written huffman tables are the default ones, this failure had
> gone undetected.
> ---
>  libavformat/rtpdec_jpeg.c |   33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/libavformat/rtpdec_jpeg.c b/libavformat/rtpdec_jpeg.c
> index 671763d..19ecbf4 100644
> --- a/libavformat/rtpdec_jpeg.c
> +++ b/libavformat/rtpdec_jpeg.c
> @@ -76,13 +76,12 @@ static void jpeg_free_context(PayloadContext *jpeg)
>      av_free(jpeg);
>  }
>
> -static void jpeg_create_huffman_table(PutBitContext *p, int table_class,
> -                                      int table_id, const uint8_t *bits_table,
> -                                      const uint8_t *value_table)
> +static int jpeg_create_huffman_table(PutBitContext *p, int table_class,
> +                                     int table_id, const uint8_t *bits_table,
> +                                     const uint8_t *value_table)

Maybe, you can use put_huffman_table() from libavcodec/mjpegenc.c now.
Martin Storsjö Sept. 11, 2012, 2:54 p.m. | #3
On Tue, 11 Sep 2012, Samuel Pitoiset wrote:

> On Tue, Sep 11, 2012 at 1:46 PM, Martin Storsjö <martin@martin.st> wrote:
>> Currently the size header of the generated DHT section is
>> incorrect, making the mjpeg decoder just skip it. Since the
>> written huffman tables are the default ones, this failure had
>> gone undetected.
>> ---
>>  libavformat/rtpdec_jpeg.c |   33 ++++++++++++++++++++-------------
>>  1 file changed, 20 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavformat/rtpdec_jpeg.c b/libavformat/rtpdec_jpeg.c
>> index 671763d..19ecbf4 100644
>> --- a/libavformat/rtpdec_jpeg.c
>> +++ b/libavformat/rtpdec_jpeg.c
>> @@ -76,13 +76,12 @@ static void jpeg_free_context(PayloadContext *jpeg)
>>      av_free(jpeg);
>>  }
>>
>> -static void jpeg_create_huffman_table(PutBitContext *p, int table_class,
>> -                                      int table_id, const uint8_t *bits_table,
>> -                                      const uint8_t *value_table)
>> +static int jpeg_create_huffman_table(PutBitContext *p, int table_class,
>> +                                     int table_id, const uint8_t *bits_table,
>> +                                     const uint8_t *value_table)
>
> Maybe, you can use put_huffman_table() from libavcodec/mjpegenc.c now.

I'd prefer not to - we don't require mjpegenc.o when using the rtp 
depacketizer right now, only mjpeg.o, and if you have a look at 
http://patches.libav.org/patch/27159/, you'll see that I change the use of 
the bitstream writer into a bytestream writer, which wouldn't work with 
that function.

// Martin
Samuel Pitoiset Sept. 11, 2012, 2:57 p.m. | #4
On Tue, Sep 11, 2012 at 4:54 PM, Martin Storsjö <martin@martin.st> wrote:
> On Tue, 11 Sep 2012, Samuel Pitoiset wrote:
>
>> On Tue, Sep 11, 2012 at 1:46 PM, Martin Storsjö <martin@martin.st> wrote:
>>>
>>> Currently the size header of the generated DHT section is
>>> incorrect, making the mjpeg decoder just skip it. Since the
>>> written huffman tables are the default ones, this failure had
>>> gone undetected.
>>> ---
>>>  libavformat/rtpdec_jpeg.c |   33 ++++++++++++++++++++-------------
>>>  1 file changed, 20 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/libavformat/rtpdec_jpeg.c b/libavformat/rtpdec_jpeg.c
>>> index 671763d..19ecbf4 100644
>>> --- a/libavformat/rtpdec_jpeg.c
>>> +++ b/libavformat/rtpdec_jpeg.c
>>> @@ -76,13 +76,12 @@ static void jpeg_free_context(PayloadContext *jpeg)
>>>      av_free(jpeg);
>>>  }
>>>
>>> -static void jpeg_create_huffman_table(PutBitContext *p, int table_class,
>>> -                                      int table_id, const uint8_t
>>> *bits_table,
>>> -                                      const uint8_t *value_table)
>>> +static int jpeg_create_huffman_table(PutBitContext *p, int table_class,
>>> +                                     int table_id, const uint8_t
>>> *bits_table,
>>> +                                     const uint8_t *value_table)
>>
>>
>> Maybe, you can use put_huffman_table() from libavcodec/mjpegenc.c now.
>
>
> I'd prefer not to - we don't require mjpegenc.o when using the rtp
> depacketizer right now, only mjpeg.o, and if you have a look at
> http://patches.libav.org/patch/27159/, you'll see that I change the use of
> the bitstream writer into a bytestream writer, which wouldn't work with that
> function.

Ok.

Patch

diff --git a/libavformat/rtpdec_jpeg.c b/libavformat/rtpdec_jpeg.c
index 671763d..19ecbf4 100644
--- a/libavformat/rtpdec_jpeg.c
+++ b/libavformat/rtpdec_jpeg.c
@@ -76,13 +76,12 @@  static void jpeg_free_context(PayloadContext *jpeg)
     av_free(jpeg);
 }
 
-static void jpeg_create_huffman_table(PutBitContext *p, int table_class,
-                                      int table_id, const uint8_t *bits_table,
-                                      const uint8_t *value_table)
+static int jpeg_create_huffman_table(PutBitContext *p, int table_class,
+                                     int table_id, const uint8_t *bits_table,
+                                     const uint8_t *value_table)
 {
     int i, n = 0;
 
-    put_bits(p, 8, 0);
     put_bits(p, 4, table_class);
     put_bits(p, 4, table_id);
 
@@ -94,12 +93,15 @@  static void jpeg_create_huffman_table(PutBitContext *p, int table_class,
     for (i = 0; i < n; i++) {
         put_bits(p, 8, value_table[i]);
     }
+    return n + 17;
 }
 
 static int jpeg_create_header(uint8_t *buf, int size, uint32_t type, uint32_t w,
                               uint32_t h, const uint8_t *qtable, int nb_qtable)
 {
     PutBitContext pbc;
+    uint8_t *dht_size_ptr;
+    int dht_size;
 
     init_put_bits(&pbc, buf, size);
 
@@ -142,15 +144,20 @@  static int jpeg_create_header(uint8_t *buf, int size, uint32_t type, uint32_t w,
 
     /* DHT */
     put_marker(&pbc, DHT);
-
-    jpeg_create_huffman_table(&pbc, 0, 0, avpriv_mjpeg_bits_dc_luminance,
-                              avpriv_mjpeg_val_dc);
-    jpeg_create_huffman_table(&pbc, 0, 1, avpriv_mjpeg_bits_dc_chrominance,
-                              avpriv_mjpeg_val_dc);
-    jpeg_create_huffman_table(&pbc, 1, 0, avpriv_mjpeg_bits_ac_luminance,
-                              avpriv_mjpeg_val_ac_luminance);
-    jpeg_create_huffman_table(&pbc, 1, 1, avpriv_mjpeg_bits_ac_chrominance,
-                              avpriv_mjpeg_val_ac_chrominance);
+    flush_put_bits(&pbc);
+    dht_size_ptr = put_bits_ptr(&pbc);
+    put_bits(&pbc, 16, 0);
+
+    dht_size  = 2;
+    dht_size += jpeg_create_huffman_table(&pbc, 0, 0,avpriv_mjpeg_bits_dc_luminance,
+                                          avpriv_mjpeg_val_dc);
+    dht_size += jpeg_create_huffman_table(&pbc, 0, 1, avpriv_mjpeg_bits_dc_chrominance,
+                                          avpriv_mjpeg_val_dc);
+    dht_size += jpeg_create_huffman_table(&pbc, 1, 0, avpriv_mjpeg_bits_ac_luminance,
+                                          avpriv_mjpeg_val_ac_luminance);
+    dht_size += jpeg_create_huffman_table(&pbc, 1, 1, avpriv_mjpeg_bits_ac_chrominance,
+                                          avpriv_mjpeg_val_ac_chrominance);
+    AV_WB16(dht_size_ptr, dht_size);
 
     /* SOF0 */
     put_marker(&pbc, SOF0);