Avidemux Forum

When I try the VDPAU output driver I just a black window.  Both paused and during playback.

I've tried both lavc decode and vdpau decode with the same result. vdpau decode with qt or xvideo output works.  I seems to be the vdpau output that's the problem.

I've build of avidemux in FC32 as well as my own build of current git.

I'd think maybe vdpau was was just broken in general on my system, but it works with mpv!

mesa-vdpau-drivers-20.1.9-1.fc32.x86_64, libvdpau-1.4-2.fc32.x86_64

AMD Radeon RX560

Any ideas where I might look for a problem?  I'll attach some logs.
You cannot view this attachment.
You cannot view this attachment.
You cannot view this attachment.

Avidemux doesn't think that anything were amiss. Do you get a picture when you preview the output of the VDPAU Resize video filter in filter manager with width and/or height modified (else the filter operates in passthrough mode)?

If this works, then the only obvious difference which I can spot at the moment is that we call VdpPresentationQueueDisplay in our admVdpau::presentationQueueDisplay wrapper with clip_width and clip_height set to zero (which is perfectly valid) while mpv seems to always use the actual dimensions of the video display.

With the size modified in vdpau resize filter, I get only a green window in filter preview.  If I leave the size unchanged, then I get a correct display.  Even though normal playback is all black.
But I don't think preview uses vdpau for display because CPU usage is too high.

I tried changing the clip size to the video size, but it didn't change anything.

It must be admVdpau::mixerRenderWithCropping with G3DVL then (VDPAU works in Avidemux with NVIDIA hardware + driver).

What if you comment out GUI_vdpauRender.cpp:214 and reactivate the line 213?

(Of course, this won't have any effect on the filter, but maybe on the display in the main window.)

Quote from: xyzzy on October 10, 2020, 10:31:01 AMBut I don't think preview uses vdpau for display because CPU usage is too high.

This was the reason I asked as this helped to narrow down the list of possible culprits.

Quote from: eumagga0x2a on October 10, 2020, 10:46:20 AMWhat if you comment out GUI_vdpauRender.cpp:214 and reactivate the line 213?

I must add that I don't believe now that this can make any difference.

Your finding is the result of a total lack of testing with AMD graphics until now.

Quote from: eumagga0x2a on October 10, 2020, 12:36:08 PM

QuoteWhat if you comment out GUI_vdpauRender.cpp:214 and reactivate the line 213?

I must add that I don't believe now that this can make any difference.

And indeed it didn't .

Thanks, please discard that change.

Could you please test whether it makes a difference when we actually pass a non-NULL destination_video_rect to VdpVideoMixerRender as mpv does? The patch is attached.

If it doesn't help, the only remaining diff to mpv I could easily spot was that mpv passes two (valid?) surfaces as each of video_surface_past/future arrays.

That fixed it!

Something I've noticed is that the video surface created for a 1280x720 video is actually 1280x736.  I wonder if that is part of the problem?

According to the docs, it should be legal to pass NULL for destination_video_rect: "If NULL, the destination rectangle will be sized to match the source rectangle, and will be located at the origin."

It's not clear to me if the "source rectangle" is the background source rectangle or the video source rectangle.  I'd think video is intended, but maybe that's not how it works.  If it was background, since there is no background rect, maybe that causes the problem?

destination_rect is also NULL, and the docs do NOT describe that as allowed.  I tried changing that, but it didn't fix the problem.  It didn't hurt to set both rects though, which seems the correct thing to do.

I haven't tried interlaced content yet, but it looks like the code paths for that might be different.

Great news, thanks! Did it fix VDPAU video filters too?

Quote from: xyzzy on October 11, 2020, 08:43:44 PMSomething I've noticed is that the video surface created for a 1280x720 video is actually 1280x736.  I wonder if that is part of the problem?

A rounding error in vdpauRender::rescaleDisplay() from scalingFactor returned by QWindow::devicePixelRatio() being slightly greater than 1.0?? I don't think that this matters, but I cannot reproduce this on my desktop. Any ideas?

Quote from: xyzzy on October 11, 2020, 08:43:44 PMAccording to the docs, it should be legal to pass NULL for destination_video_rect: "If NULL, the destination rectangle will be sized to match the source rectangle, and will be located at the origin."

I stumbled over this statement too. The wording is really slippery as "destination rectangle" is likely NOT the destination_video_rect this section explains but destination_rect, the previous parameter.

Quote from: xyzzy on October 11, 2020, 08:43:44 PMdestination_rect is also NULL, and the docs do NOT describe that as allowed.

It is exactly the other way round IMHO, mpv did it right, we were wrong.

I found the code in mesa that handles this.

It should be fine to leave destination_video_rect as NULL as it will set it to video_source_rect.  I had a link to the exact line of code in the mesa git here, but cleantalk keeps rejecting my message. It seems like it is not functional for a technical forum.

There was a bug in mesa and it didn't do this.  It was fixed four months ago, and my mesa was released last week, but the fix is only on the mesa 20.2.x branch on not mesa 20.1.x branch.

So that's the bug, it's actually in mesa.  From the fix commit message (b2324f45), it looks like mpv used to do the same thing as avidemux is doing.

It does looks like leaving destination_rect as NULL is not a good idea.  It's not documented as ok and it doesn't appear the mesa code specifically handles that case.  It only works because the various places that get passed that rect all appear to work ok with NULL.

Take a look at src gallium frontends vdpau mixer.c on line 334.  Maybe this post will be allowed.

That CleanTalk thing seems to be completely untunable and erratic. It catches the most of spam, but side effects are awful.

Thank you for pointing me to frontends/vdpau: Default destination rect to source rect, quite remarkable circular workaround-chasing.

Quote from: eumagga0x2a on October 11, 2020, 09:48:44 PMA rounding error in vdpauRender::rescaleDisplay() from scalingFactor returned by QWindow::devicePixelRatio() being slightly greater than 1.0??

I think it's just an artifact of how the hardware does the video decode.  Maybe it needs to be a multiple of 32?  The call to admVdpau::surfaceCreate() is for 1280x720, but the surface created is 1280x736.  I think this is fine.  But I wonder if NULL rects everywhere at some point causes the full video surface, 736 lines, to be used when it should be cropped to 720.

Quote from: xyzzy on October 11, 2020, 11:45:06 PMThe call to admVdpau::surfaceCreate() is for 1280x720, but the surface created is 1280x736.

I cannot reproduce this behavior on my system so far. Did you catch it with something like

diff --git a/avidemux_core/ADM_coreVideoCodec/ADM_hwAccel/ADM_coreVdpau/src/ADM_coreVdpau.cpp b/avidemux_core/ADM_coreVideoCodec/ADM_hwAccel/ADM_coreVdpau/src/ADM_coreVdpau.cpp
index c54529488..f1ffabd53 100644
--- a/avidemux_core/ADM_coreVideoCodec/ADM_hwAccel/ADM_coreVdpau/src/ADM_coreVdpau.cpp
+++ b/avidemux_core/ADM_coreVideoCodec/ADM_hwAccel/ADM_coreVdpau/src/ADM_coreVdpau.cpp
@@ -309,6 +309,16 @@ VdpStatus  admVdpau::surfaceCreate(uint32_t width,uint32_t height,VdpVideoSurfac
        ADM_warning("ADM_coreVdpau::funcs.createSurface(ADM_coreVdpau::vdpDevice,VDP_CHROMA_TYPE_420,width,height,surface) call failed with error=%s\n",getErrorString(r));
        return r;
    }
+    // check that we got what we asked for
+    VdpChromaType chroma;
+    uint32_t actualWidth,actualHeight;
+    if(VDP_STATUS_OK != ADM_coreVdpau::funcs.mixerGetSurfaceParameters(*surface, &chroma, &actualWidth, &actualHeight))
+    {
+        ADM_warning("mixerGetSurfaceParameters failed\n");
+    }else if(actualWidth != widthToUse || actualHeight != heightToUse)
+    {
+        ADM_warning("Requested %ux%u but surface created as %ux%u\n",widthToUse,heightToUse,actualWidth,actualHeight);
+    }
    myIterator already = listOfAllocatedSurface.find(*surface);
    if(already!=listOfAllocatedSurface.end())
    {

?

QuoteI cannot reproduce this behavior on my system so far. Did you catch it with something like

Yes, exactly like that.  I expect there is a hardware dependent multiple of 32 requirement somewhere, so if your hardware only requires a multiple of 16, then you wouldn't see it.  It doesn't seem to be a problem.  The mixer render calls specify a video source rect of the correct size so I assume the padding gets dropped like it should.