On Sat, Nov 03, 2012 at 10:15:45PM +0100, Jordi Ortiz wrote: > --- > Changelog | 2 +- > configure | 1 + > doc/general.texi | 4 +- > libavcodec/Makefile | 2 + > libavcodec/allcodecs.c | 1 + > libavcodec/dirac_arith.c | 115 +++ > libavcodec/dirac_arith.h | 166 ++++ > libavcodec/diracdec.c | 2029 > ++++++++++++++++++++++++++++++++++++++++++++++ > 8 files changed, 2317 insertions(+), 3 deletions(-) > create mode 100644 libavcodec/dirac_arith.c > create mode 100644 libavcodec/dirac_arith.h > create mode 100644 libavcodec/diracdec.c
The version bump is missing, see http://www.libav.org/developer.html#New-codecs-or-formats-checklist Does the Dirac decoder compile standalone? It would be nice to get FATE tests as followup commits. > --- a/Changelog > +++ b/Changelog > @@ -58,7 +58,7 @@ version 9_beta1: > - Opus decoder and encoder using libopus > - remove -same_quant, it hasn't worked for years > - support for building with MSVC > - > +- Native dirac decoder > > version 0.8: The double empty line was there on purpose. > --- a/doc/general.texi > +++ b/doc/general.texi > @@ -475,8 +475,8 @@ following image formats are supported: > @item Creative YUV (CYUV) @tab @tab X > @item DFA @tab @tab X > @tab Codec used in Chronomaster game. > -@item Dirac @tab E @tab E > - @tab supported through external library libschroedinger > +@item Dirac @tab E @tab X > + @tab supported through external library libschroedinger and native > decoder This is somewhat unclear, @tab encoding supported through external library libschroedinger, decoding supported through either libschroedinger or native decoder BTW, have you benchmarked our decoder against libdirac or libschroedinger? > --- a/libavcodec/Makefile > +++ b/libavcodec/Makefile > @@ -136,6 +136,8 @@ OBJS-$(CONFIG_CYUV_DECODER) += cyuv.o > OBJS-$(CONFIG_DFA_DECODER) += dfa.o > +OBJS-$(CONFIG_DIRAC_DECODER) += diracdec.o dirac.o diracdsp.o \ > + dirac_arith.o mpeg12data.o > dirac_dwt.o Move mpeg12data.o to the end of the line. > --- /dev/null > +++ b/libavcodec/diracdec.c > @@ -0,0 +1,2029 @@ > + > +/** > + * The spec limits the number of wavelet decompositions to 4 for both > + * level 1 (VC-2) and 128 (long-gop default). > + * 5 decompositions is the maximum before >16-bit buffers are needed. > + * Schroedinger allows this for DD 9,7 and 13,7 wavelets only, limiting > + * the others to 4 decompositions (or 3 for the fidelity filter). > + * > + * We use this instead of MAX_DECOMPOSITIONS to save some memory. > + */ > +#define DIRAC_MAX_DWT_LEVELS 5 Why are this and many other comments Doxygen? They look file-specific to me... > +typedef struct DiracContext { > + /** > + * Schroedinger older than 1.0.8 doesn't store > + * quant delta if only one codebook exists in a band > + */ > + unsigned old_delta_quant; > + unsigned codeblock_mode; All other comments on this struct are not Doxygen ... > +/* > + * [DIRAC_STD] 13.4.3.2 Codeblock unpacking loop. codeblock() trailing whitespace > +static int dirac_unpack_prediction_parameters(DiracContext *s) > +{ > + if (idx == 0) { > + s->plane[0].xblen = svq3_get_ue_golomb(gb); > + s->plane[0].yblen = svq3_get_ue_golomb(gb); > + s->plane[0].xbsep = svq3_get_ue_golomb(gb); > + s->plane[0].ybsep = svq3_get_ue_golomb(gb); unrelated: I keep wondering why this function has a "svq3_" prefix. > + /* [DIRAC_STD] 11.2.8 Reference picture weight. > reference_picture_weights() > + * just data read, weight calculation will be done later on. */ > + s->weight_log2denom = 1; > + s->weight[0] = 1; > + s->weight[1] = 1; The comment is ungrammatical. What does "just data read" mean here? > + /* [DIRAC_STD] 11.3.5 Quantization matrices (low-delay syntax). > + * quant_matrix() */ > + if (get_bits1(gb)) { > + av_log(s->avctx, AV_LOG_DEBUG, > + "Low Delay: Has Custom Quantization Matrix!\n"); Is there a reason for the odd capitalization? > +static void global_mv(DiracContext *s, DiracBlock *block, int x, int y, > + int ref) > +{ > + int ez = s->globalmc[ref].zrs_exp; > + int ep = s->globalmc[ref].perspective_exp; > + int (*A)[2] = s->globalmc[ref].zrs; > + int *b = s->globalmc[ref].pan_tilt; > + int *c = s->globalmc[ref].perspective; Align all the =. > +/* > + * Copy the current block to the other blocks covered by the current > + * superblock split mode > + */ > +static void propagate_block_data(DiracBlock *block, int stride, int size) nit: You could squash this comment to two lines. > +/* > + * For block x,y, determine which of the hpel planes to do bilinear > + * interpolation from and set src[] to the location in each hpel plane > + * to MC from. > + * > + * @return the index of the put_dirac_pixels_tab function to use > + * 0 for 1 plane (fpel,hpel), 1 for 2 planes (qpel), 2 for 4 planes (qpel), > + * and 3 for epel > + */ > +static int mc_subpel(DiracContext *s, DiracBlock *block, const uint8_t > *src[5], > + int x, int y, int ref, int plane) You use @return syntax, but this is not a Doxygen comment. > + if (!s->num_refs) { /* intra */ > + for (y = 0; y < p->height; y += 16) { > + ff_spatial_idwt_slice2(&d, y + 16); /* decode */ > + s->diracdsp.put_signed_rect_clamped(frame + y * p->stride, > + p->stride, > + p->idwt_buf > + + y * p->idwt_stride, > + p->idwt_stride, p->width, > + 16); Move the '+' to the previous line. > +AVCodec ff_dirac_decoder = { > + .name = "dirac", > + .type = AVMEDIA_TYPE_VIDEO, > + .id = CODEC_ID_DIRAC, AV_CODEC_ID_DIRAC Diego _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
