search

ros-misc-utilities / ffmpeg_image_transport

ROS2 image transport plugin for encoding/decoding with h264 codec
Apache License 2.0
65 stars 22 forks source link

encoder: fix strides when bgr is target format #5

Closed akssri-sony closed 1 year ago

akssri-sony commented 1 year ago

Apologies; there was another bug in the previous PR. Also changed the target fmt from AV_PIX_FMT_BGR0 (4-bytes per pixel position) to AV_PIX_FMT_BGR24.

(BTW, are there encoders that directly encode BGR24 ? Maybe by going to YUV422 ? )

berndpfrommer commented 1 year ago

Your question leaves me confused. It seemed to me like your conditional checking for BGR24 was added precisely because you had an encoder that worked directly off of BGR24. Is that not so? Then how did you test the code?

akssri-sony commented 1 year ago

I haven't tested this code path.

  1. The pix_fmt BGR0 used in the original code is in fact 32bpp, but the memcpy was assuming 24bpp which is BGR24.
  2. There was a bug in the previous PR where the bpp was assumed to be 8 instead of 24 as before.

This PR fixes both 1. and 2.

berndpfrommer commented 1 year ago

I tested on a mix of ROS2 galactic and ROS2 humble using nvenc and it works. Not sure what code paths have been tested, but at least for me there was no regression. Thanks again for the fixes!