[MPlayer-dev-eng] [PATCH] vd_null to output black frames

[Xidorn Quan]

On Sat, Nov 3, 2012 at 9:20 AM, Alexander Strasser <eclipse7 at gmx.net> wrote:

> Alexander Strasser wrote:
> > Xidorn Quan wrote:
> > > On Fri, Nov 2, 2012 at 4:56 AM, Reimar Döffinger wrote:
> > > > On Thu, Nov 01, 2012 at 09:51:09PM +0100, Alexander Strasser wrote:
> > > > >
> > > > >   I was thinking if "dummy" is expressive enough as a name for a
> > > > configuration
> > > > > flag. Would "ignore_fourcc" be easier to understand for someone
> reading
> > > > codecs.conf?
> > > > >
> > > > >   I do not particularly care about the naming but I do like the
> patch.
> > > > So please
> > > > > do not understand this as a blocking issue. Just wanted to find
> out what
> > > > other
> > > > > developers think.
> > > >
> > > > It's more that I thought we might want to use it for other stuff, for
> > > > example not bothering with copying around the actual data of the
> > > > packets, or running a parser in certain cases or such.
> > > > Which is why I'd currently be more in favour of "dummy" than
> something
> > > > specific.
> > > > But I don't dare to try to predict if that will really make sense in
> the
> > > > long run.
> > > >
> > >
> > > In my opinion, a "dummy" codec is such a codec created for testing,
> > > demonstrating, or whatever, so it should never be chosen if not be
> > > forced by user. For which, we mark them as "crashing" at present, but
> > > actually, both null and black are working as expected accurately.
> > > I prefer to mark them "working" with the flag "dummy", and prevent
> > > a codec flagged that to be chosen when it is not forced.
> > >
> > > I agree that "ignore_fourcc" is a more expressive name for the
> > > behavior in my patch. However, if we make the "dummy" flag work as
> > > what I have mentioned above, we do not need the "ignore_fourcc" flag
> > > anymore, since user is responsible for taking care of fourcc if he
> > > forces such codecs.
> >
> >   OK, thanks for explaining your motivations. Combined this gives
> > good reason to use the name "dummy". Some other name that came
> > to my mind is "fake" which probably more accurately describes the
> > nature of those "codecs". (But maybe it sounds to negative?)
> >
> >   To restate it I am not against using "dummy". Never was.
> >
> >   I also like Xidorn's idea of never choosing the decoder because it
> > is flagged "dummy" and having the status set to "working" again for
> > null and black. It would eliminate the misuse of the status property.
> >
> >   Reimar, what do you think?
>
>   Attached is a *not* very well tested RFC patch to implement the
> behavior Xidorn described above. It must be applied incrementally
> on top of Xidorn's patch.
>
>   I am unsure if this change of behavior has any additional (high
> level) consequences I am not aware of.


Well, in fact I have implemented this behavior in my local repo, and
just forgot to send the patch.

The patch attached includes my prevous patch, and it implements the
behavior in a different way. I think it should be more safe to keep
changes inside codec-cfg instead of modifying both dec_audio and
dec_video.

Regards,
Xidorn Quan
-------------- next part --------------
Index: codec-cfg.c
===================================================================
--- codec-cfg.c	(revision 35336)
+++ codec-cfg.c	(working copy)
@@ -752,6 +752,9 @@
             if (!strcmp(token[0], "align16"))
                 codec->flags |= CODECS_FLAG_ALIGN16;
             else
+            if (!strcmp(token[0], "dummy"))
+                codec->flags |= CODECS_FLAG_DUMMY;
+            else
                 goto err_out_parse_error;
         } else if (!strcmp(token[0], "status")) {
             if (get_token(1, 1) < 0)
@@ -869,9 +872,10 @@
         if(!i) return NULL;
         for (/* NOTHING */; i--; c++) {
             if(start && c<=start) continue;
+            if (!force && c->flags & CODECS_FLAG_DUMMY)
+                continue;
             for (j = 0; j < CODECS_MAX_FOURCC; j++) {
-                // FIXME: do NOT hardwire 'null' and 'black' here:
-                if (c->fourcc[j]==fourcc || !strcmp(c->drv,"null") || !strcmp(c->drv,"black")) {
+                if (c->fourcc[j]==fourcc || c->flags & CODECS_FLAG_DUMMY) {
                     if (fourccmap)
                         *fourccmap = c->fourccmap[j];
                     return c;
Index: codec-cfg.h
===================================================================
--- codec-cfg.h	(revision 35336)
+++ codec-cfg.h	(working copy)
@@ -26,6 +26,7 @@
 // Global flags:
 #define CODECS_FLAG_SEEKABLE    (1<<0)
 #define CODECS_FLAG_ALIGN16     (1<<1)
+#define CODECS_FLAG_DUMMY       (1<<2)
 
 // Outfmt flags:
 #define CODECS_FLAG_FLIP        (1<<0)
Index: etc/codecs.conf
===================================================================
--- etc/codecs.conf	(revision 35337)
+++ etc/codecs.conf	(working copy)
@@ -4215,9 +4215,10 @@
 
 videocodec null
   info "NULL codec (no decoding!)"
-  status crashing
+  status working
   comment "for unknown/unsupported codecs or testing"
   driver null
+  flags dummy
   out YV12
   out I420
   out YUY2
@@ -4227,9 +4228,10 @@
 
 videocodec black
   info "black codec (no decoding just output black frames!)"
-  status crashing
+  status working
   comment "for unknown/unsupported codecs or testing"
   driver black
+  flags dummy
   out YV12
   out I420
   out YUY2