video/image_writer: tag bits_per_raw_sample when pixfmt changes #11247
Conversation
| * ignore this value, but some codecs can make use of it (for example, PNG's | ||
| * sBIT chunk or JXL's bit depth header) | ||
| */ | ||
| if (ctx->opts->high_bit_depth && ctx->original_format.id != image->imgfmt) { |
There was a problem hiding this comment.
why only if the pixfmt is different?
There was a problem hiding this comment.
If the pixel format is the same, then there's no need for this metadata as it would set it to the max of the pixel format.
If you think it would still be valuable to set anyway, then I can remove that check.
There was a problem hiding this comment.
That's fine it just tripped me up.
There was a problem hiding this comment.
One thing to note is that ctx->opts->high_bit_depth is no guarantee that you get a bit depth > 8, so you may need to adjust the check.
because bits_per_raw_sample shouldn't be set for 8-bit screenshots.
btw, why is that?
There was a problem hiding this comment.
The idea was that higher bit depth video can be tagged appropriately (e.g. 10bit) although I suppose theoretically you could have a lower bit depth PNG or JXL than 8.
There was a problem hiding this comment.
Ah, I thought there was a technical reason why that must not happen not just "it's pointless".
re the problem I mentioned: drop the ctx->opts->high_bit_depth check then?
This populates the AVCodecContext->bits_per_raw_sample field value when the pixel format of the screenshot being written doesn't match the pixel format of the original video. For example, if the original video was 10-bit and the screenshot is 16-bit, it populates this field so the encoder can make use of it. Currently only the libjxl encoder respects this tag although PNG supports sBIT. Once lavc's PNG encoder respects this tag it will work seamlessly with this commit as well. This feature is only enabled if screenshot-high-bit-depth=yes is enabled, because bits_per_raw_sample shouldn't be set for 8-bit screenshots.
|
Update: This only appears to work when |
|
That is to be expected since hardware screenshots are downloaded as RGB0 (8bpc) or RGBA64 (16bpc) and that is considered the "source" format. |
This populates the AVCodecContext->bits_per_raw_sample field value when the pixel format of the screenshot being written doesn't match the pixel format of the original video. For example, if the original video was 10-bit and the screenshot is 16-bit, it populates this field so the encoder can make use of it.
Currently only the libjxl encoder respects this tag although PNG supports sBIT. Once lavc's PNG encoder respects this tag it will work seamlessly with this commit as well.
This feature is only enabled if screenshot-high-bit-depth=yes is enabled, because bits_per_raw_sample shouldn't be set for 8-bit screenshots.