On Thu, Feb 01, 2018 at 01:43:00PM +0100, Jerome Martinez wrote: > Add support for 16-bit/component RGB with Alpha encoding and decoding in > FFV1, both RGBA64 and GBRAP16 for encoding, GBRAP16 for decoding. > > Resulting bitstream was tested about lossless encoding/decoding by the > compression from DPX to FFV1 then decompression from FFV1 to DPX, see > commands below (resulting framemd5 hashes are all same). > Resulting bitstream is decodable by another decoder (with same resulting > framemd5 hash). > Resulting bitstream passed through a conformance checker compared to current > FFV1 specification IETF draft. > > About the patch: > - some modified lines are not used (the ones not used when f->use32bit is > 1), but it makes the code more coherent (especially because decode_rgb_frame > signature is same for both 16-bit and 32-bit version) and prepares the > support of RGBA with 10/12/14 bits/component. > - GBRAP16 was chosen for decoding because GBRP16 is already used when no > alpha, and the code is more prepared for planar pix_fmt when bit depth is > >8. > - "s->transparency = desc->nb_components == 4 || desc->nb_components == 2;" > is a copy of a line a bit above about the detection of transparency, I > preferred to reuse it as is even if "YA" 16-bit/component is not (yet) > supported. > > FFmpeg commands used for tests: > ./ffmpeg -i in.dpx -c:v ffv1 out.mkv > ./ffmpeg -i in.dpx -pix_fmt gbrap16 -strict -2 -c:v ffv1 out2.mkv > ./ffmpeg -i out.mkv out.dpx > > ./ffmpeg -i in.dpx -f framemd5 in.dpx.framemd5 > ./ffmpeg -i out.mkv -pix_fmt rgba64be -f framemd5 out.mkv.framemd5 > ./ffmpeg -i out2.mkv -pix_fmt rgba64be -f framemd5 out2.mkv.framemd5 > ./ffmpeg -i out.dpx -f framemd5 out.dpx.framemd5 > > Test file used (renamed to in.dpx): > https://mediaarea.net/temp/uncropped_DPX_4K_16bit_Overscan15pros.dpx > > Jérôme >
[...]
> diff --git a/libavcodec/ffv1enc_template.c b/libavcodec/ffv1enc_template.c
> index b7eea0dd70..2763082540 100644
> --- a/libavcodec/ffv1enc_template.c
> +++ b/libavcodec/ffv1enc_template.c
> @@ -122,8 +122,8 @@ static av_always_inline int
> RENAME(encode_line)(FFV1Context *s, int w,
> return 0;
> }
>
> -static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[3],
> - int w, int h, const int stride[3])
> +static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[4],
> + int w, int h, const int stride[4])
> {
> int x, y, p, i;
> const int ring_size = s->context_model ? 3 : 2;
> @@ -152,14 +152,18 @@ static int RENAME(encode_rgb_frame)(FFV1Context *s,
> const uint8_t *src[3],
> r = (v >> 16) & 0xFF;
> a = v >> 24;
> } else if (packed) {
> - const uint16_t *p = ((const uint16_t*)(src[0] + x*6 +
> stride[0]*y));
> + const uint16_t *p = ((const uint16_t*)(src[0] + x*(3 +
> s->transparency)*2 + stride[0]*y));
> r = p[0];
> g = p[1];
> b = p[2];
> + if (s->transparency)
transparency should possibly be moved into a local variable
> + a = p[3];
> } else if (sizeof(TYPE) == 4) {
> g = *((const uint16_t *)(src[0] + x*2 + stride[0]*y));
> b = *((const uint16_t *)(src[1] + x*2 + stride[1]*y));
> r = *((const uint16_t *)(src[2] + x*2 + stride[2]*y));
> + if (s->transparency)
> + a = *((const uint16_t *)(src[3] + x*2 + stride[2]*y));
^^^^^^^^^
this looks wrong
also what speed effect do thes changes to the inner loop have ?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The educated differ from the uneducated as much as the living from the
dead. -- Aristotle
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
