[FFmpeg-devel] Patch: CrystalHD decoder support
Philip Langdale
philipl
Tue Dec 28 18:46:17 CET 2010
On Tue, 28 Dec 2010 10:03:25 +0000 (UTC)
Carl Eugen Hoyos <cehoyos at ag.or.at> wrote:
> 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?
Yes, although it's a highly questionable activity; The hardware can only
decode to YUY2, so you waste your time converting back to YUV420 and I
doubt it's a lossless conversion.
> ...
>
> > -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?
It isn't. That's leftover from my previous iteration as an hwaccel using
Gwenole's v2 patch. It turns out this is not a good fit for the
interface so I redid it as a full decoder.
> ...
>
> > + * - 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...
Sorry - I was sloppy here; the function originally comes from
gstbcmdec (the gstreamer filter) which is LGPL. I will change the
credits accordingly.
> ...
>
> > +#define _GNU_SOURCE
>
> I suspect you should explain why this is needed.
It's needed to get usleep(). I will add a comment.
> ...
>
> > +#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.
Do you think there's insufficient benefit in fast_memcpy to bother
taking advantage of it? I'll remove the usage if you don't want it
but it seems desirable to use it if it's there.
>
> > +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.
Will do.
> ...
>
> > + for (unsigned int i = 0; i < num_pps; i++)
> > + {
>
> On one line.
Will do. This is the original code style of the function.
> > + if (data_size < 2) {
> > + return -1;
> > + }
>
> Most people here think that the braces are unneeded and reduce
> readability.
Will do.
Thanks,
--phil
More information about the ffmpeg-devel
mailing list