Jürgen Meissner wrote:
vf_logo.c implements avfilters "logo" and "logo_without_alpha"
Great! This is one of the feature most requested by users!
logo=10:20:logo.jpg overlays the video at x=10 y=20 with the logo (is
respecting the alpha-path of the logo)
logo=-1:1:logo.png overlays the video at 1 pixel from the right and 1
pixel from the top border
logo_without_alpha=10:20:logo.gif overlays the video at 10,20 for all
pixels of the logo, doesn't look for an alpha-path in the logo
logo=0:0:0:10:20:logo.gif overlays video at 10,20 and assumes black
RGB=(0,0,0) is the transparent color in the logo
vf_logo.c is relying on the new pixdesc.c rather than the old pix-fmt.c
and therefor needs pixdesc.o to be included into the libavcodec.a archive.
I'll add a few comments to those Setefano gave.
03_libavcodec_Makefile.diff adds the pixdesc.o to the OBS list in the
libavcodec/Makefile
I leave this to Stefano/Michael...
+/**
+ * RGBA pixel.
+ */
+typedef struct {
+ uint8_t R; ///< Red.
+ uint8_t G; ///< Green.
+ uint8_t B; ///< Blue.
+ uint8_t A; ///< Alpha.
+} RGBA;
+
We usually do not use those type of structs, but just uint8_t[4].
+static const char *pixdesc_name(int pix_fmt)
+{
+ return av_pix_fmt_descriptors[pix_fmt].name;
+}
+
I think you can just inline this function where it is needed.
+ if (logo->pCodecCtx->pix_fmt != PIX_FMT_RGBA) {
+ // transform it with swscaler to PIX_FMT_RGBA
It seems that you always convert the logo to RGBA. This is a bad idea
because if both the logo and the movie is in YUV format, the logo will
be converted YUV->RGB->YUV, that would degrade the logo quality.
+ // Allocate an AVFrame structure
+ logo->plogo_frame_rgba32 = avcodec_alloc_frame();
What this comment says is already obvious from the line it describes,
making it useless and distracting (that probably happens elsewhere in
the code too).
+static void uninit(AVFilterContext * ctx)
+{
+
+ LogoContext *logo;
+
+ logo = ctx->priv;
+
+ if (logo->sws != NULL)
+ sws_freeContext(logo->sws);
I think that if you do colorspace conversion just once, there is no need
to having logo->sws in the context (and that would simplify
allocation/freeing).
+static AVFilterFormats *make_format_list(LogoContext * logo)
+{
+ AVFilterFormats *ret;
+ int i;
+
+ ret = av_mallocz(sizeof(AVFilterFormats));
+ ret->formats = av_malloc(sizeof(int) * PIX_FMT_NB);
+
+ for (i = 0; i < PIX_FMT_NB; i++) {
+ switch (i) {
+ /* don't support these */
+ case PIX_FMT_YUYV422:
+ case PIX_FMT_MONOBLACK:
+ case PIX_FMT_UYVY422:
+ break;
+ /* support everything else (if named) */
+ default:
+ if (av_pix_fmt_descriptors[i].name)
+ ret->formats[ret->format_count++] = i;
+ }
+ }
+ return ret;
While I agree with Stefano that it would be hard to do less ugly, what
bothers me about this code is that it is fragile. It is very likely it
will break when new formats are added....
Thanks for your contribution!
-Vitor
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
