diff mbox series

[FFmpeg-devel,v2] lavf/rtpenc_jpeg: Retrieve sampling factor from SOF0

Message ID CY4PR11MB203804C465FFB9AC9D44C74BC1FC9@CY4PR11MB2038.namprd11.prod.outlook.com
State New
Headers show
Series [FFmpeg-devel,v2] lavf/rtpenc_jpeg: Retrieve sampling factor from SOF0 | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Yeoh, Hoong Tee Jan. 11, 2023, 1:35 a.m. UTC 
In ff_rtp_send_jpeg, the type is defined based on PIX_FMT and
color-range parsed in. There is limitation on current design
where need to include support newly introduced PIX_FMT such as
AV_PIX_FMT_QSV and there might be more and more in future. Hence,
retrive the sampling factor from SOF0 in JPEG compressed header
directly. This introduces flexibility to handle different type of
new codec introduced in future.

Signed-off-by: Yeoh, Hoong Tee <hoong.tee.yeoh@intel.com>
---
 libavformat/rtpenc_jpeg.c | 41 +++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 15 deletions(-)

Comments

Xiang, Haihao June 12, 2023, 2:46 a.m. UTC | #1 
On Wo, 2023-01-11 at 01:35 +0000, Yeoh, Hoong Tee wrote:
> In ff_rtp_send_jpeg, the type is defined based on PIX_FMT and
> color-range parsed in. There is limitation on current design
> where need to include support newly introduced PIX_FMT such as
> AV_PIX_FMT_QSV and there might be more and more in future. Hence,
> retrive the sampling factor from SOF0 in JPEG compressed header
> directly. This introduces flexibility to handle different type of
> new codec introduced in future.
> 
> Signed-off-by: Yeoh, Hoong Tee <hoong.tee.yeoh@intel.com>
> ---
>  libavformat/rtpenc_jpeg.c | 41 +++++++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/libavformat/rtpenc_jpeg.c b/libavformat/rtpenc_jpeg.c
> index 9111683..53be188 100644
> --- a/libavformat/rtpenc_jpeg.c
> +++ b/libavformat/rtpenc_jpeg.c
> @@ -30,7 +30,7 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t
> *buf, int size)
>      RTPMuxContext *s = s1->priv_data;
>      const uint8_t *qtables[4] = { NULL };
>      int nb_qtables = 0;
> -    uint8_t type;
> +    uint8_t type = 2; /* initialized non-0/1 value for RTP/JPEG type check*/
>      uint8_t w, h;
>      uint8_t *p;
>      int off = 0; /* fragment offset of the current JPEG frame */
> @@ -45,20 +45,6 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t
> *buf, int size)
>      w = AV_CEIL_RSHIFT(s1->streams[0]->codecpar->width, 3);
>      h = AV_CEIL_RSHIFT(s1->streams[0]->codecpar->height, 3);
>  
> -    /* get the pixel format type or fail */
> -    if (s1->streams[0]->codecpar->format == AV_PIX_FMT_YUVJ422P ||
> -        (s1->streams[0]->codecpar->color_range == AVCOL_RANGE_JPEG &&
> -         s1->streams[0]->codecpar->format == AV_PIX_FMT_YUV422P)) {
> -        type = 0;
> -    } else if (s1->streams[0]->codecpar->format == AV_PIX_FMT_YUVJ420P ||
> -               (s1->streams[0]->codecpar->color_range == AVCOL_RANGE_JPEG &&
> -                s1->streams[0]->codecpar->format == AV_PIX_FMT_YUV420P)) {
> -        type = 1;
> -    } else {
> -        av_log(s1, AV_LOG_ERROR, "Unsupported pixel format\n");
> -        return;
> -    }
> -
>      /* preparse the header for getting some info */
>      for (i = 0; i < size; i++) {
>          if (buf[i] != 0xff)
> @@ -90,6 +76,23 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t
> *buf, int size)
>                         "Only 1x1 chroma blocks are supported. Aborted!\n");
>                  return;
>              }
> +
> +           /*
> +            * Find out the sampling factor in SOF0.
> +            * In SOF0, hsample/vsample is inserted in form of (2<<4) | (type
> ? 2 : 1).
> +            * First 4-bit is hsample while Last 4-bit is vsample.
> +            */
> +
> +            /* Luma channel sampling factor in 4:2:2 chroma subsampling are
> 2x1 */
> +            if (buf[i + 11] == 33) {
> +                type = 0;
> +            /* Luma channel sampling factor in 4:2:0 chroma subsampling are
> 2x2 */
> +            } else if (buf[i + 11] == 34) {
> +                type = 1;
> +            } else {
> +                av_log(s1, AV_LOG_ERROR, "Unsupported pixel format\n");
> +                return;
> +            }
>          } else if (buf[i + 1] == DHT) {
>              int dht_size = AV_RB16(&buf[i + 2]);
>              default_huffman_tables |= 1 << 4;
> @@ -163,6 +166,14 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t
> *buf, int size)
>              break;
>          }
>      }
> +
> +    /* Check validity of RTP/JPEG type */
> +    if (type != 0 && type != 1) {
> +        av_log(s1, AV_LOG_ERROR,
> +                "Invalid RTP/JPEG type\n");
> +        return;
> +    }
> +
>      if (default_huffman_tables && default_huffman_tables != 31) {
>          av_log(s1, AV_LOG_ERROR,
>                 "RFC 2435 requires standard Huffman tables for jpeg\n");


Sorry to pick up this old thread. This patch looks good and works well for me. I
will merge it if no one objects in a few days. 

Thanks
Haihao
Xiang, Haihao June 20, 2023, 1:55 a.m. UTC | #2 
On Ma, 2023-06-12 at 02:46 +0000, Xiang, Haihao wrote:
> On Wo, 2023-01-11 at 01:35 +0000, Yeoh, Hoong Tee wrote:
> > In ff_rtp_send_jpeg, the type is defined based on PIX_FMT and
> > color-range parsed in. There is limitation on current design
> > where need to include support newly introduced PIX_FMT such as
> > AV_PIX_FMT_QSV and there might be more and more in future. Hence,
> > retrive the sampling factor from SOF0 in JPEG compressed header
> > directly. This introduces flexibility to handle different type of
> > new codec introduced in future.
> > 
> > Signed-off-by: Yeoh, Hoong Tee <hoong.tee.yeoh@intel.com>
> > ---
> >  libavformat/rtpenc_jpeg.c | 41 +++++++++++++++++++++++++--------------
> >  1 file changed, 26 insertions(+), 15 deletions(-)
> > 
> > diff --git a/libavformat/rtpenc_jpeg.c b/libavformat/rtpenc_jpeg.c
> > index 9111683..53be188 100644
> > --- a/libavformat/rtpenc_jpeg.c
> > +++ b/libavformat/rtpenc_jpeg.c
> > @@ -30,7 +30,7 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t
> > *buf, int size)
> >      RTPMuxContext *s = s1->priv_data;
> >      const uint8_t *qtables[4] = { NULL };
> >      int nb_qtables = 0;
> > -    uint8_t type;
> > +    uint8_t type = 2; /* initialized non-0/1 value for RTP/JPEG type
> > check*/
> >      uint8_t w, h;
> >      uint8_t *p;
> >      int off = 0; /* fragment offset of the current JPEG frame */
> > @@ -45,20 +45,6 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t
> > *buf, int size)
> >      w = AV_CEIL_RSHIFT(s1->streams[0]->codecpar->width, 3);
> >      h = AV_CEIL_RSHIFT(s1->streams[0]->codecpar->height, 3);
> >  
> > -    /* get the pixel format type or fail */
> > -    if (s1->streams[0]->codecpar->format == AV_PIX_FMT_YUVJ422P ||
> > -        (s1->streams[0]->codecpar->color_range == AVCOL_RANGE_JPEG &&
> > -         s1->streams[0]->codecpar->format == AV_PIX_FMT_YUV422P)) {
> > -        type = 0;
> > -    } else if (s1->streams[0]->codecpar->format == AV_PIX_FMT_YUVJ420P ||
> > -               (s1->streams[0]->codecpar->color_range == AVCOL_RANGE_JPEG
> > &&
> > -                s1->streams[0]->codecpar->format == AV_PIX_FMT_YUV420P)) {
> > -        type = 1;
> > -    } else {
> > -        av_log(s1, AV_LOG_ERROR, "Unsupported pixel format\n");
> > -        return;
> > -    }
> > -
> >      /* preparse the header for getting some info */
> >      for (i = 0; i < size; i++) {
> >          if (buf[i] != 0xff)
> > @@ -90,6 +76,23 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t
> > *buf, int size)
> >                         "Only 1x1 chroma blocks are supported. Aborted!\n");
> >                  return;
> >              }
> > +
> > +           /*
> > +            * Find out the sampling factor in SOF0.
> > +            * In SOF0, hsample/vsample is inserted in form of (2<<4) |
> > (type
> > ? 2 : 1).
> > +            * First 4-bit is hsample while Last 4-bit is vsample.
> > +            */
> > +
> > +            /* Luma channel sampling factor in 4:2:2 chroma subsampling are
> > 2x1 */
> > +            if (buf[i + 11] == 33) {
> > +                type = 0;
> > +            /* Luma channel sampling factor in 4:2:0 chroma subsampling are
> > 2x2 */
> > +            } else if (buf[i + 11] == 34) {
> > +                type = 1;
> > +            } else {
> > +                av_log(s1, AV_LOG_ERROR, "Unsupported pixel format\n");
> > +                return;
> > +            }
> >          } else if (buf[i + 1] == DHT) {
> >              int dht_size = AV_RB16(&buf[i + 2]);
> >              default_huffman_tables |= 1 << 4;
> > @@ -163,6 +166,14 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const
> > uint8_t
> > *buf, int size)
> >              break;
> >          }
> >      }
> > +
> > +    /* Check validity of RTP/JPEG type */
> > +    if (type != 0 && type != 1) {
> > +        av_log(s1, AV_LOG_ERROR,
> > +                "Invalid RTP/JPEG type\n");
> > +        return;
> > +    }
> > +
> >      if (default_huffman_tables && default_huffman_tables != 31) {
> >          av_log(s1, AV_LOG_ERROR,
> >                 "RFC 2435 requires standard Huffman tables for jpeg\n");
> 
> 
> Sorry to pick up this old thread. This patch looks good and works well for me.
> I
> will merge it if no one objects in a few days. 
> 

Applied, thx.
diff mbox series

Patch

diff --git a/libavformat/rtpenc_jpeg.c b/libavformat/rtpenc_jpeg.c
index 9111683..53be188 100644
--- a/libavformat/rtpenc_jpeg.c
+++ b/libavformat/rtpenc_jpeg.c
@@ -30,7 +30,7 @@  void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size)
     RTPMuxContext *s = s1->priv_data;
     const uint8_t *qtables[4] = { NULL };
     int nb_qtables = 0;
-    uint8_t type;
+    uint8_t type = 2; /* initialized non-0/1 value for RTP/JPEG type check*/
     uint8_t w, h;
     uint8_t *p;
     int off = 0; /* fragment offset of the current JPEG frame */
@@ -45,20 +45,6 @@  void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size)
     w = AV_CEIL_RSHIFT(s1->streams[0]->codecpar->width, 3);
     h = AV_CEIL_RSHIFT(s1->streams[0]->codecpar->height, 3);
 
-    /* get the pixel format type or fail */
-    if (s1->streams[0]->codecpar->format == AV_PIX_FMT_YUVJ422P ||
-        (s1->streams[0]->codecpar->color_range == AVCOL_RANGE_JPEG &&
-         s1->streams[0]->codecpar->format == AV_PIX_FMT_YUV422P)) {
-        type = 0;
-    } else if (s1->streams[0]->codecpar->format == AV_PIX_FMT_YUVJ420P ||
-               (s1->streams[0]->codecpar->color_range == AVCOL_RANGE_JPEG &&
-                s1->streams[0]->codecpar->format == AV_PIX_FMT_YUV420P)) {
-        type = 1;
-    } else {
-        av_log(s1, AV_LOG_ERROR, "Unsupported pixel format\n");
-        return;
-    }
-
     /* preparse the header for getting some info */
     for (i = 0; i < size; i++) {
         if (buf[i] != 0xff)
@@ -90,6 +76,23 @@  void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size)
                        "Only 1x1 chroma blocks are supported. Aborted!\n");
                 return;
             }
+
+	    /*
+	     * Find out the sampling factor in SOF0.
+	     * In SOF0, hsample/vsample is inserted in form of (2<<4) | (type ? 2 : 1).
+	     * First 4-bit is hsample while Last 4-bit is vsample.
+	     */
+
+            /* Luma channel sampling factor in 4:2:2 chroma subsampling are 2x1 */
+            if (buf[i + 11] == 33) {
+                type = 0;
+            /* Luma channel sampling factor in 4:2:0 chroma subsampling are 2x2 */
+            } else if (buf[i + 11] == 34) {
+                type = 1;
+            } else {
+                av_log(s1, AV_LOG_ERROR, "Unsupported pixel format\n");
+                return;
+            }
         } else if (buf[i + 1] == DHT) {
             int dht_size = AV_RB16(&buf[i + 2]);
             default_huffman_tables |= 1 << 4;
@@ -163,6 +166,14 @@  void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size)
             break;
         }
     }
+
+    /* Check validity of RTP/JPEG type */
+    if (type != 0 && type != 1) {
+        av_log(s1, AV_LOG_ERROR,
+                "Invalid RTP/JPEG type\n");
+        return;
+    }
+
     if (default_huffman_tables && default_huffman_tables != 31) {
         av_log(s1, AV_LOG_ERROR,
                "RFC 2435 requires standard Huffman tables for jpeg\n");