Thanks for the review
On 21/01/13 22:55, Diego Biurrun wrote:
> Thanks for getting back to this decoder!
>
> On Mon, Jan 21, 2013 at 08:25:39PM +0100, Jordi Ortiz wrote:
>> ---
>> Use VideoDSPContext.emulated_edge_mc() instead of ff_emultated_edge_mc_8()
>> Use the ff_get_buffer() wrapper.
>>
>> Changelog | 1 +
>> configure | 1 +
>> doc/general.texi | 5 +-
>> libavcodec/Makefile | 3 +
>> libavcodec/allcodecs.c | 1 +
>> libavcodec/dirac_arith.c | 115 +++
>> libavcodec/dirac_arith.h | 166 ++++
>> libavcodec/diracdec.c | 2014
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> libavcodec/version.h | 2 +-
>> 9 files changed, 2305 insertions(+), 3 deletions(-)
>> create mode 100644 libavcodec/dirac_arith.c
>> create mode 100644 libavcodec/dirac_arith.h
>> create mode 100644 libavcodec/diracdec.c
> What is the status of this decoder now? Does it still produce artifacts?
> If yes, for which cases?
The status is the same as it was. When sub pixel motion vector precision
is used the decoded result isn't bit exact with last libschroedinger
version.
Also deep-estimation is not working properly. If anyone knows what
deep-estimation is and wants to point it out it would be very welcome, I
didn't manage to find any reference in the standard. I know that deep
estimation uses mv_precision values over zero so the problem in this
case could be related to the mv_precision.
>> --- /dev/null
>> +++ b/libavcodec/dirac_arith.h
>> @@ -0,0 +1,166 @@
>> +
>> +#ifndef AVCODEC_DIRAC_ARITH_H
>> +#define AVCODEC_DIRAC_ARITH_H
>> +
>> +#include "bytestream.h"
>> +#include "get_bits.h"
> This does not appear to use anything from get_bits.h.
Moved the include to dirac_arith.c. There GetBitContext is used to
calculate how much data is left.
>> --- /dev/null
>> +++ b/libavcodec/diracdec.c
>> @@ -0,0 +1,2014 @@
>> +
>> +static void dirac_decode_flush(AVCodecContext *avctx)
>> +{
>> + DiracContext *s = avctx->priv_data;
>> + free_sequence_buffers(s);
>> + s->seen_sequence_header = 0;
>> + s->frame_number = -1;
>> +}
>> +
>> +static av_cold int dirac_decode_end(AVCodecContext *avctx)
>> +{
>> + dirac_decode_flush(avctx);
>> + return 0;
>> +}
> Why the indirection?
Might it be because AVCodec.close and AVCodec.flush have different
return types?
>> +/* [DIRAC_STD] 11.2 Picture prediction data. picture_prediction()
>> + * Unpack the motion compensation parameters. */
>> +static int dirac_unpack_prediction_parameters(DiracContext *s)
>> +{
>> + if (FFMAX(s->plane[0].xblen, s->plane[0].yblen) > DIRAC_MAX_BLOCKSIZE) {
>> + av_log(s->avctx, AV_LOG_ERROR, "Unsupported large block size\n");
>> + return AVERROR_PATCHWELCOME;
> Is this a candidate for av_log_missing_feature or av_log_patch_welcome?
av_log_missing_feature sounds better.
Should I change all the av_log() calls to av_log_... calls?
>> +/* [DIRAC_STD] 11.1.1 Picture Header. picture_header() */
>> +static int dirac_decode_picture_header(DiracContext *s)
>> +{
>> + int retire, picnum;
>> + int i, j, refnum, refdist, ret, distance;
>> + GetBitContext *gb = &s->gb;
>> +
>> + /* [DIRAC_STD] 11.1.1 Picture Header. picture_header() PICTURE_NUM */
>> + picnum =
>> + s->current_picture->avframe.display_picture_number =
>> + get_bits_long(gb, 32);
>> + picnum = s->current_picture->avframe.display_picture_number =
>> get_bits_long(gb, 32);
> nit:
>
> picnum =
> s->current_picture->avframe.display_picture_number = get_bits_long(gb,
> 32);
aargh, I dislike this one. Is it mandatory? it looks horrible to me
--
Jordi Ortiz
[email protected]
http://www.jordiortiz.es
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
