vf_stereo3d: fix "auto" input format with libav

[ghedo]

I'm not 100% sure the fix is correct (i.e. if the NULL was on purpose), but it seems to work.

Also, maybe the two ifs can be merged like so:

diff --git a/video/filter/vf_stereo3d.c b/video/filter/vf_stereo3d.c
index e5a84f4..e572476 100644
--- a/video/filter/vf_stereo3d.c
+++ b/video/filter/vf_stereo3d.c
@@ -541,17 +541,14 @@ static int vf_open(vf_instance_t *vf)
     if (vf->priv->in.fmt == STEREO_AUTO)
         vf->priv->auto_in = 1;

-    if (vf->priv->in.fmt == STEREO_AUTO &&
-        vf_lw_set_graph(vf, vf->priv->lw_opts, "stereo3d", "null") >= 0)
-    {
-        vf_lw_set_reconfig_cb(vf, lavfi_reconfig);
-        return 1;
-    }
-
     if (vf_lw_set_graph(vf, vf->priv->lw_opts, "stereo3d", "%s:%s",
                         rev_map_name(vf->priv->in.fmt),
                         rev_map_name(vf->priv->out.fmt)) >= 0)
+    {
+        if (vf->priv->in.fmt == STEREO_AUTO)
+            vf_lw_set_reconfig_cb(vf, lavfi_reconfig);
         return 1;
+    }

     return 1;
 }

but I haven't tested if this works with ffmpeg.

[ghedo]

Btw, this was reported at https://bugs.debian.org/763904

[ghost]

I'm not sure if this is a bug. Yes, the filter is auto-inserted, and if you specify an additional filter on the command line, it will be inserted twice and applied twice. mpv --vf=stereo3d=sbs2l:ml test.mk3d --video-stereo-mode=none works fine for me, so does mpv test.mk3d.

I'd call this a user error.

Your PR doesn't actually change anything for me. And Libav doesn't provide a stereo3d filter, so the internal one is used. (I'd like to remove the internal one, btw., to reduce the mess.)

I'm not sure about your second suggested patch; may or may not work depending on things.

[ghedo]

And Libav doesn't provide a stereo3d filter, so the internal one is used

That's the point, this doesn't work because the call to vf_lw_set_graph() has NULL instead of the filter name (streo3d), so mpv thinks that the vf_stereo3d libavfilter filter exists while in fact it doesn't (so later lavfi_reconfig is called, which fails). The error I get with libav is:

     libav: AVFilterGraph: No such filter: 'stereo3d'
  stereo3d: Can't configure libavfilter graph.
        vf: Video filter chain:
        vf:  [vd] 720x408->725x408 yuv420p Autoselect/Autoselect CL=1
        vf:  [in] 720x408 yuv420p BT.601 (SD)/TV CL=1
        vf:  [stereo3d] 720x408 yuv420p BT.601 (SD)/TV CL=1   <---
        vf:  [out] 720x408 yuv420p BT.601 (SD)/TV CL=1
        vd: Cannot initialize video filters.
   cplayer: Can't insert 3D conversion filter.

by replacing NULL with "stereo3d", the first vf_lw_set_graph fails with libav so that the lavfi_reconfig callback is not enabled and the internal filter is used.

[ghedo]

Actually, I'm not sure if it's lavfi_reconfig() that fails, but the first call to vf_lw_set_graph() should (like the second one), so that the internal filter is used.

[ghost]

Ah, I see... that's nasty. I see how that would fix it (I didn't actually test with Libav), so ok to merge. I think you actually have push access? If so, feel free to push.

Btw., now that ffmpeg is in debian, couldn't you package mpv against ffmpeg?

[ghedo]

If so, feel free to push.

Done. Btw, I tried the other patch with ffmpeg and it doesn't work, so there's that (it's because the version of ffmpeg I have doesn't support "auto", but the libavfilter vf_stereo3d is still inserted).

Btw., now that ffmpeg is in debian, couldn't you package mpv against ffmpeg?

The ffmpeg package is still only in experimental at the moment. There have been talks about replacing libav with ffmpeg in Debian, but that's been put on hold and postponed to after the upcoming Debian release (which at best will take a few months to actually be released).

In any case I was looking into ways to provide both mpv linked to ffmpeg in experimental, and to libav for testing and the next stable release, at least untill the libav vs ffmpeg thing is resolved.

[ghost]

it's because the version of ffmpeg I have doesn't support "auto"

Yes, "auto" is a mpv-internal thing. "auto" is replaced with the format hint stored in mp_image_params, which originates from file tags or similar.

The ffmpeg package is still only in experimental at the moment.

I think it made it into unstable.

[ghedo]

Yes, "auto" is a mpv-internal thing. "auto" is replaced with the format hint stored in mp_image_params, which originates from file tags or similar

Ah, ok, then I guess it doesn't do the translation with that patch, because I get some errors like:

    ffmpeg: Eval: Undefined constant or missing '(' in 'auto'
    ffmpeg: stereo3d: Unable to parse option value "auto"
    ffmpeg: Eval: Undefined constant or missing '(' in 'auto'
    ffmpeg: stereo3d: Unable to parse option value "auto"
    ffmpeg: stereo3d: Error setting option in to value auto.
    ffmpeg: Parsed_stereo3d_0: Error applying options to the filter.
    ffmpeg: ?: Error initializing filter 'stereo3d' with args 'auto:ml'
  stereo3d: Can't configure libavfilter graph.
        vf: Video filter chain:
        vf:  [vd] 720x408->725x408 yuv420p Autoselect/Autoselect CL=1
        vf:  [in] 720x408 yuv420p BT.601 (SD)/TV CL=1
        vf:  [stereo3d] 720x408 yuv420p BT.601 (SD)/TV CL=1   <---
        vf:  [out] 720x408 yuv420p BT.601 (SD)/TV CL=1
        vd: Cannot initialize video filters.
   cplayer: Can't insert 3D conversion filter.

I think it made it into unstable.

Oh right, I missed that. Still, due to https://bugs.debian.org/763148 (it's not really a bug, but the Debian security team doesn't want to have to deal with both libav and ffmpeg in the stable release, and at this point it's too late to switch to ffmpeg) the ffmpeg packages aren't going to be in the next Debian release, So if mpv used them it would be left out as well.

Anyway, I just uploaded mpv 0.6.0-1+ffmpeg (built against the ffmpeg) to experimental so that's at least something for now.

[ghost]

Ah, ok, then I guess it doesn't do the translation with that patch, because I get some errors like:

So it's broken now?

I just uploaded mpv 0.6.0-1+ffmpeg (built against the ffmpeg) to experimental

That's pretty nice.

[ghedo]

So it's broken now?

No, the other patch above (merging the ifs) breaks things. I tried it with ffmpeg and it doesn't work so you can just ignore it.