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

[Xiang, Haihao]

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

Applied, thx.