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

[Xiang, Haihao]

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 at 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