[FFmpeg-devel] [PATCH 2/2] diracdec: Add HQ Profile support
Michael Niedermayer
michaelni at gmx.at
Tue Dec 8 23:30:53 CET 2015
On Tue, Dec 08, 2015 at 07:27:42PM +0000, Kieran Kunhya wrote:
> From: Rostislav Pehlivanov <atomnuker at gmail.com>
>
> ---
> libavcodec/dirac.c | 31 ++--
> libavcodec/dirac.h | 28 ++++
> libavcodec/dirac_dwt.h | 2 +-
> libavcodec/diracdec.c | 357 +++++++++++++++++++++++++++-----------------
> libavformat/oggparsedirac.c | 3 +-
> 5 files changed, 265 insertions(+), 156 deletions(-)
>
> diff --git a/libavcodec/dirac.c b/libavcodec/dirac.c
> index aa82dd9..67447c5 100644
> --- a/libavcodec/dirac.c
> +++ b/libavcodec/dirac.c
> @@ -109,10 +109,11 @@ static const struct {
> { AVCOL_PRI_BT709, AVCOL_SPC_BT709, AVCOL_TRC_UNSPECIFIED /* DCinema */ },
> };
>
> -/* [DIRAC_STD] Table 10.2 Supported chroma sampling formats + luma Offset */
> -static const enum AVPixelFormat dirac_pix_fmt[2][3] = {
> - { AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV420P },
> - { AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ420P },
> +/* [DIRAC_STD] Table 10.2 Supported chroma sampling formats */
> +static const enum AVPixelFormat dirac_pix_fmt[3][3] = {
[][3] should avoid hardcoding the array size
[...]
> diff --git a/libavcodec/dirac_dwt.h b/libavcodec/dirac_dwt.h
> index 41842b5..e885373 100644
> --- a/libavcodec/dirac_dwt.h
> +++ b/libavcodec/dirac_dwt.h
> @@ -24,7 +24,7 @@
> #include <stdint.h>
>
> typedef int DWTELEM;
> -typedef short IDWTELEM;
> +typedef int IDWTELEM;
If its easy to seperate this out then it would allow improved
bisectability
[...]
> @@ -224,17 +235,6 @@ typedef struct DiracContext {
> DiracFrame all_frames[MAX_FRAMES];
> } DiracContext;
>
> -/**
> - * Dirac Specification ->
> - * Parse code values. 9.6.1 Table 9.1
> - */
> -enum dirac_parse_code {
> - pc_seq_header = 0x00,
> - pc_eos = 0x10,
> - pc_aux_data = 0x20,
> - pc_padding = 0x30,
> -};
> -
> enum dirac_subband {
> subband_ll = 0,
> subband_hl = 1,
unrelated, this is already unused before
[...]
> @@ -466,7 +482,7 @@ static av_cold int dirac_decode_end(AVCodecContext *avctx)
>
> #define SIGN_CTX(x) (CTX_SIGN_ZERO + ((x) > 0) - ((x) < 0))
>
> -static inline int coeff_unpack_golomb(GetBitContext *gb, int qfactor, int qoffset)
> +static av_always_inline int coeff_unpack_golomb(GetBitContext *gb, int qfactor, int qoffset)
> {
> int sign, coeff;
>
> @@ -584,18 +600,6 @@ static inline void codeblock(DiracContext *s, SubBand *b,
> }
> }
>
> -#define PARSE_VALUES(type, x, gb, ebits, buf1, buf2) \
> - type *buf = (type *)buf1; \
> - buf[x] = coeff_unpack_golomb(gb, qfactor, qoffset); \
> - if (get_bits_count(gb) >= ebits) \
> - return; \
> - if (buf2) { \
> - buf = (type *)buf2; \
> - buf[x] = coeff_unpack_golomb(gb, qfactor, qoffset); \
> - if (get_bits_count(gb) >= ebits) \
> - return; \
> - } \
> -
> /**
> * Dirac Specification ->
> * 13.3 intra_dc_prediction(band)
> @@ -719,49 +723,68 @@ static void decode_component(DiracContext *s, int comp)
> avctx->execute(avctx, decode_subband_golomb, bands, NULL, num_bands, sizeof(SubBand*));
> }
>
> -/* [DIRAC_STD] 13.5.5.2 Luma slice subband data. luma_slice_band(level,orient,sx,sy) --> if b2 == NULL */
> -/* [DIRAC_STD] 13.5.5.3 Chroma slice subband data. chroma_slice_band(level,orient,sx,sy) --> if b2 != NULL */
> -static void lowdelay_subband(DiracContext *s, GetBitContext *gb, int quant,
> - int slice_x, int slice_y, int bits_end,
> - SubBand *b1, SubBand *b2)
> +#define PARSE_VALUES(type, x, gb, ebits, buf1, buf2) \
PARSE_VALUES was just added in the previous patch, why is it moved
around after its addition ?
It would be cleaner to place it initially whereever its final location
is intended to be
[...]
> - for (y = top; y < bottom; y++) {
> - for (x = left; x < right; x++) {
> - if (s->pshift) {
> + if (s->pshift) {
> + for (y = top; y < bottom; y++) {
> + for (x = left; x < right; x++) {
> PARSE_VALUES(int32_t, x, gb, bits_end, buf1, buf2);
> - } else {
> + }
> +
> + buf1 += b1->stride;
> + if (buf2)
> + buf2 += b2->stride;
> + }
> + }
> + else {
> + for (y = top; y < bottom; y++) {
> + for (x = left; x < right; x++) {
> PARSE_VALUES(int16_t, x, gb, bits_end, buf1, buf2);
> }
> +
> + buf1 += b1->stride;
> + if (buf2)
> + buf2 += b2->stride;
> }
> - buf1 += b1->stride;
> - if (buf2)
> - buf2 += b2->stride;
> }
> }
>
this would be more readable without the indention change
> -struct lowdelay_slice {
> +/* Used by Low Delay and High Quality profiles */
> +typedef struct DiracSlice {
> GetBitContext gb;
> int slice_x;
> int slice_y;
> int bytes;
> -};
> -
> +} DiracSlice;
> /**
> * Dirac Specification ->
> @@ -770,7 +793,7 @@ struct lowdelay_slice {
> static int decode_lowdelay_slice(AVCodecContext *avctx, void *arg)
> {
> DiracContext *s = avctx->priv_data;
> - struct lowdelay_slice *slice = arg;
> + DiracSlice *slice = arg;
> GetBitContext *gb = &slice->gb;
> enum dirac_subband orientation;
> int level, quant, chroma_bits, chroma_end;
> @@ -784,8 +807,8 @@ static int decode_lowdelay_slice(AVCodecContext *avctx, void *arg)
> for (level = 0; level < s->wavelet_depth; level++)
> for (orientation = !!level; orientation < 4; orientation++) {
> quant = FFMAX(quant_base - s->lowdelay.quant[level][orientation], 0);
> - lowdelay_subband(s, gb, quant, slice->slice_x, slice->slice_y, luma_end,
> - &s->plane[0].band[level][orientation], NULL);
> + decode_subband(s, gb, quant, slice->slice_x, slice->slice_y, luma_end,
> + &s->plane[0].band[level][orientation], NULL);
> }
>
> /* consume any unused bits from luma */
> @@ -797,10 +820,48 @@ static int decode_lowdelay_slice(AVCodecContext *avctx, void *arg)
> for (level = 0; level < s->wavelet_depth; level++)
> for (orientation = !!level; orientation < 4; orientation++) {
> quant = FFMAX(quant_base - s->lowdelay.quant[level][orientation], 0);
> - lowdelay_subband(s, gb, quant, slice->slice_x, slice->slice_y, chroma_end,
> - &s->plane[1].band[level][orientation],
> - &s->plane[2].band[level][orientation]);
> + decode_subband(s, gb, quant, slice->slice_x, slice->slice_y, chroma_end,
> + &s->plane[1].band[level][orientation],
> + &s->plane[2].band[level][orientation]);
These changes seem purely cosmetical, shouldnt these renamings be
in a seperate patch ?
it seems the code also crashes: (cant immedeatly see in the patch what
causes this)
Program received signal SIGFPE, Arithmetic exception.
0x000000000075956d in decode_lowdelay (s=0x1d45000) at libavcodec/diracdec.c:899
899 bytes = (slice_num+1) * s->lowdelay.bytes.num / s->lowdelay.bytes.den
(gdb) bt
#0 0x000000000075956d in decode_lowdelay (s=0x1d45000) at libavcodec/diracdec.c:899
#1 0x000000000075ceea in dirac_decode_frame_internal (s=0x1d45000) at libavcodec/diracdec.c:1740
#2 0x000000000075e154 in dirac_decode_data_unit (avctx=0x1d4b280, buf=0x1d610b0 "BBCD\214", size=5409) at libavcodec/diracdec.c:2064
#3 0x000000000075e3c4 in dirac_decode_frame (avctx=0x1d4b280, data=0x1d6a0c0, got_frame=0x7fffffffdcd4, pkt=0x7fffffffd9b0) at libavcodec/diracdec.c:2118
#4 0x0000000000b18899 in avcodec_decode_video2 (avctx=0x1d4b280, picture=0x1d6a0c0, got_picture_ptr=0x7fffffffdcd4, avpkt=0x7fffffffdc10) at libavcodec/utils.c:2098
#5 0x0000000000431ff0 in decode_video (ist=0x1d4b060, pkt=0x7fffffffdc10, got_output=0x7fffffffdcd4) at ffmpeg.c:2090
#6 0x0000000000433132 in process_input_packet (ist=0x1d4b060, pkt=0x7fffffffddf0, no_eof=0) at ffmpeg.c:2339
#7 0x000000000043a246 in process_input (file_index=0) at ffmpeg.c:3979
#8 0x000000000043a552 in transcode_step () at ffmpeg.c:4067
#9 0x000000000043a699 in transcode () at ffmpeg.c:4121
#10 0x000000000043ad7a in main (argc=6, argv=0x7fffffffe488) at ffmpeg.c:4314
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151208/a32fbd4b/attachment.sig>
More information about the ffmpeg-devel
mailing list