Total duration in filter preview after changeFps

Started by eumagga0x2a, January 05, 2017, 11:42:22 AM

Previous topic - Next topic

eumagga0x2a

Would it be possible to find time to investigate the issue

QuoteThe total duration accessed as _in->getInfo()->totalDuration is not updated if the changeFps filter changing the duration is added. This might be an issue with underlying code and not the preview dialog's fault.

mentioned in https://github.com/mean00/avidemux2/pull/64 in near future?

Apart from that, it would be immensly helpful if releases were announced a bit in advance, this would save packagers including myself quite a lot of easily avoidable trouble. Thanks!

mean

It has been done in a hurry, the preview bug was really bad
I will not have much time for a few days, so it was now or much later

eumagga0x2a

No problem with that, thank you very much for providing solutions that quickly and subsequent speedy bugfix-releases, but it would have been helpful to know that these releases were imminent. In this case I wouldn't have tried to backport >10 post-2.6.15 patches and wouldn't have pushed 2.6.16b to be accepted by RPMFusion for Fedora, then stopping them until the worst issues are fixed upstream, then merging the commits and releasing a patched 2.6.16 for Fedora only to fall one commit short of 2.6.17.

I'll try to look into this issue myself, without any hope to come far, but it will be fun anyway  :)

eumagga0x2a

Actually, there is a major issue with 2.6.17 which was not present in 2.6.16 + all later commits except of [filters] Make the preview modal and clean it up, to avoid using stale handles later on: the navigation slider in the common filter preview (the one launched from the video filter manager) doesn't work. Likely related to the cleanup in Q_seekablePreview.{cpp,h}.

eumagga0x2a

Yes, reverting the changes to Q_seekablePreview.cpp and Q_seekablePreview.h fixes the navigation slider in the common filter preview window.

mean

I was expecting problems, i hate rushing release like that

So it will be 2.6.18 finally
My last slot is tomorrow morning

mean

The modal change is needed
Without it, try this :

* Add 2 filters (2x flip vertical for example)
* PReview first filter
* Close window
* Preview 2nd filter => slider does not work, nothing works

eumagga0x2a

Just the removal of Ui_seekablePreviewWindow::nextImage and Ui_seekablePreviewWindow::sliderChanged causes issues, the modality is fine and necessary.

eumagga0x2a

Oh, right, just Ui_seekablePreviewWindow::sliderChanged was required, [Preview] Put back slider signal. Should be managed by flydialog itself fixes the slider. Thanks!

mean


eumagga0x2a

You're right, a very poor analysis on my part. Adding debug output showing the value returned by _in->getInfo()->totalDuration proves that it is correct after changeFps. Now for the real issue:

STR:

  • Load a video, select a codec != copy, add changeFps as filter, doubling the FPS of the video to half its duration.
  • From the video filter manager, open the preview.
  • Drag the navigation slider half-way to the right
  • Drag the slider fully to the right

Actual Results:
The very end or a position close to the end of the video is reached after the Step 3, remains unchanged after Step 4.

Expected Results:
The Step 3 navigates to ~50% of the video, the Step 4 to the end.

I'm sorry, I didn't have time to look into the issue and probably won't have much time for Avidemux this week.

eumagga0x2a

#11
Oh well, removing the reimplemented goToTime function from changeFps

diff --git a/avidemux_plugins/ADM_videoFilters6/changeFps/changeFps.cpp b/avidemux_plugins/ADM_videoFilters6/changeFps/changeFps.cpp
index a12aa93..6b53273 100644
--- a/avidemux_plugins/ADM_videoFilters6/changeFps/changeFps.cpp
+++ b/avidemux_plugins/ADM_videoFilters6/changeFps/changeFps.cpp
@@ -59,7 +59,6 @@ protected:
public:
                             changeFps(ADM_coreVideoFilter *previous,CONFcouple *conf);
                             ~changeFps();
-        bool                goToTime(uint64_t usSeek);
         virtual const char   *getConfiguration(void);                   /// Return  current configuration as a human readable string
         virtual bool         getNextFrame(uint32_t *fn,ADMImage *image);    /// Return the next image
         virtual bool         getCoupledConf(CONFcouple **couples) ;   /// Return the current filter configuration
@@ -115,21 +114,6 @@ changeFps::~changeFps()
}

/**
-    \fn goToTime
-    \brief called when seeking. Need to cleanup our stuff.
-*/
-bool         changeFps::goToTime(uint64_t usSeek)
-{
-    double timing=(double)usSeek;
-    timing/=configuration.oldFpsNum;
-    timing/=configuration.newFpsDen;
-    timing*=configuration.newFpsNum;
-    timing*=configuration.oldFpsDen;
-    if(false==ADM_coreVideoFilter::goToTime((uint64_t)timing)) return false;
-    return true;
-}
-
-/**
     \fn getCoupledConf
*/
bool         changeFps::getCoupledConf(CONFcouple **couples)


would fix the issue in the filter preview, but would require all filters later in the chain using the new timing, which may be inconvenient in some cases (edit: it looks like they have to anyway).