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 (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!
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
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 :)
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 (https://github.com/mean00/avidemux2/commit/d49cac3871c2d1bbf52d0dc31a1a77800d0090b2): 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}.
Yes, reverting the changes to Q_seekablePreview.cpp and Q_seekablePreview.h fixes the navigation slider in the common filter preview window.
I was expecting problems, i hate rushing release like that
So it will be 2.6.18 finally
My last slot is tomorrow morning
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
Just the removal of Ui_seekablePreviewWindow::nextImage and Ui_seekablePreviewWindow::sliderChanged causes issues, the modality is fine and necessary.
Oh, right, just Ui_seekablePreviewWindow::sliderChanged was required, [Preview] Put back slider signal. Should be managed by flydialog itself (https://github.com/mean00/avidemux2/commit/7ec2d0b3d5b11768bdf8448ec206c2d9509ccfd5) fixes the slider. Thanks!
I cannot replicate the issue with change fps
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.
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).
Properly fixed by [changeFps] Fix changeFps gototime[/quote], thanks! (https://github.com/mean00/avidemux2/commit/2b6dd157d4fef93ad65e4948401401cdbdfcead0)