[FFmpeg-devel] Patch: CrystalHD decoder support

[Carl Eugen Hoyos]

Philip Langdale <philipl <at> overt.org> writes:

...

> I'm attaching both the main ffmpeg patch and the mplayer patch required
> to use it from mplayer.

To reduce the confusion, I would suggest to remove the MPlayer part.
Transcoding with ffmpeg works, no?

...

> -HEADERS = avcodec.h avfft.h dxva2.h opt.h vaapi.h vdpau.h xvmc.h
> +HEADERS = avcodec.h avfft.h dxva2.h hwaccel.h opt.h vaapi.h vdpau.h xvmc.h

How is this related?

...

> + * - CrystalHD decoder module -
> + *
> + * Copyright(C) 2010 Philip Langdale <ffmpeg.philipl <at> overt.org>
> + *
> + * Credits:
> + * extract_sps_pps_from_avcc: from xbmc
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public

This is the only serious part of my comments here:
Did you ask somebody from xbmc if you may re-license extract_sps_pps_from_avcc?
We try very hard to hunt copyright violators, it would be unfortunate if it also
happened to us...

...

> +#define _GNU_SOURCE

I suspect you should explain why this is needed.

...

> +#ifdef CONFIG_FASTMEMCPY
> +#include "libvo/fastmemcpy.h"
> +#else
> +#define fast_memcpy(d, s, l) memcpy(d, s, l)
> +#endif

I would suggest to remove this hunk.


> +static inline int receive_frame(AVCodecContext *avctx, void *data,
> +                                int *data_size);
> +static inline int copy_frame(AVCodecContext *avctx, BC_DTS_PROC_OUT *output,
> +                             void *data, int *data_size);

Please re-order your functions to make this unneeded.

...

> +    for (unsigned int i = 0; i < num_pps; i++)
> +    {

On one line.

> +        if (data_size < 2) {
> +            return -1;
> +        }

Most people here think that the braces are unneeded and reduce readability.

Carl Eugen

...