GNOME.org
 

[pitivi] timeline: Fix audio waveform offset

commit fb894c19abc986ad1832e2a92b38b57991687b61
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Sun Sep 19 23:03:15 2021 +0200

    timeline: Fix audio waveform offset
    
    Fix a rounding error when calculating the pixel where the start of the
    rendered sample is on the timeline (`offset_px`).
    
    Fixes #2574

 pitivi/timeline/elements.py   | 26 ++++++++---------
 pitivi/timeline/previewers.py | 47 ++++++++++++++++++-------------
 pitivi/timeline/timeline.py   |  1 +
 pitivi/utils/timeline.py      |  4 ++-
 tests/common.py               | 11 ++++++--
 tests/test_medialibrary.py    |  2 +-
 tests/test_previewers.py      | 65 ++++++++++++++++++++++++++++++++++++++++++-
 7 files changed, 117 insertions(+), 39 deletions(-)
---
diff --git a/pitivi/timeline/elements.py b/pitivi/timeline/elements.py
index 289902d8f..57f873dbd 100644
--- a/pitivi/timeline/elements.py
+++ b/pitivi/timeline/elements.py
@@ -666,9 +666,9 @@ class TimelineElement(Gtk.Layout, Zoomable, Loggable):
 
         self.props.vexpand = True
 
-        self.__previewer = self._get_previewer()
-        if self.__previewer:
-            self.add(self.__previewer)
+        self.previewer = self._get_previewer()
+        if self.previewer:
+            self.add(self.previewer)
 
         self.__background = self._get_background()
         if self.__background:
@@ -690,12 +690,12 @@ class TimelineElement(Gtk.Layout, Zoomable, Loggable):
 
     def update_previewer(self):
         """Refreshes the previewer widget."""
-        if self.__previewer:
-            self.__previewer.refresh()
+        if self.previewer:
+            self.previewer.refresh()
 
     def release(self):
-        if self.__previewer:
-            self.__previewer.release()
+        if self.previewer:
+            self.previewer.release()
 
         if self.markers:
             self._ges_elem.markers_manager.set_markers_box(None)
@@ -807,8 +807,8 @@ class TimelineElement(Gtk.Layout, Zoomable, Loggable):
     def do_draw(self, cr):
         self.propagate_draw(self.__background, cr)
 
-        if self.__previewer:
-            self.propagate_draw(self.__previewer, cr)
+        if self.previewer:
+            self.propagate_draw(self.previewer, cr)
 
         if self.keyframe_curve and self.keyframe_curve.is_drawable():
             project = self.timeline.app.project_manager.current_project
@@ -824,8 +824,8 @@ class TimelineElement(Gtk.Layout, Zoomable, Loggable):
                 selected and len(self.timeline.selection) == 1:
             self.__create_keyframe_curve()
 
-        if self.__previewer:
-            self.__previewer.set_selected(selected)
+        if self.previewer:
+            self.previewer.set_selected(selected)
 
         self.update_sizes_and_positions()
 
@@ -837,8 +837,8 @@ class TimelineElement(Gtk.Layout, Zoomable, Loggable):
         if self.__background:
             self.__background.set_size_request(width, height)
 
-        if self.__previewer:
-            self.__previewer.set_size_request(width, height)
+        if self.previewer:
+            self.previewer.set_size_request(width, height)
 
         if self.markers:
             self.markers.set_size_request(width, markers_height)
diff --git a/pitivi/timeline/previewers.py b/pitivi/timeline/previewers.py
index 24a56f7a7..82fbe213f 100644
--- a/pitivi/timeline/previewers.py
+++ b/pitivi/timeline/previewers.py
@@ -1164,9 +1164,9 @@ class AudioPreviewer(Gtk.Layout, Previewer, Zoomable, Loggable):
         self.surface = None
         # The zoom level when self.surface has been created.
         self._surface_zoom_level = 0
-        # The samples range used when self.surface has been created.
-        self._surface_start_ns = 0
-        self._surface_end_ns = 0
+        # The pixels range self.surface corresponds to.
+        self._surface_start_px = 0
+        self._surface_end_px = 0
         # The playback rate from last time the surface was updated.
         self._rate = 1.0
 
@@ -1278,47 +1278,54 @@ class AudioPreviewer(Gtk.Layout, Previewer, Zoomable, Loggable):
             # Nothing to draw.
             return
 
-        # The area we have to refresh is determined by the start and end
-        # calculated in the context of the asset duration.
-        rect = Gdk.cairo_get_clip_rectangle(context)[1]
-        clip = self.ges_elem.get_parent()
+        # The area we have to refresh is this rect inside the clip.
+        # For example rect.x is > 0 when the start of the clip is out of view.
+        # rect.width = how many pixels of the clip are in view horizontally.
+        res, rect = Gdk.cairo_get_clip_rectangle(context)
+        assert res
+
         start = self.ges_elem.props.start
         inpoint = self.ges_elem.props.in_point
         duration = self.ges_elem.props.duration
-        max_duration = self.ges_elem.get_asset().get_filesource_asset().get_duration()
-        start_ns = min(max(0, self.pixel_to_ns(rect.x) + inpoint), max_duration)
-        end_ns = min(max(0, self.pixel_to_ns(rect.x + rect.width) + inpoint), max_duration)
 
         # Get the overall rate of the clip in the current area the clip is used
+        clip = self.ges_elem.get_parent()
         internal_end = clip.get_internal_time_from_timeline_time(self.ges_elem, start + duration)
         internal_duration = internal_end - inpoint
         rate = internal_duration / duration
 
+        inpoint_px = self.ns_to_pixel(start) - self.ns_to_pixel(start - inpoint / rate)
+        max_duration_px = self.ns_to_pixel(clip.maxduration)
+
+        start_px = min(max(0, inpoint_px + rect.x), max_duration_px)
+        end_px = min(max(0, inpoint_px + rect.x + rect.width), max_duration_px)
+
         zoom = self.get_current_zoom_level()
         height = self.get_allocation().height - 2 * CLIP_BORDER_WIDTH
 
         if not self.surface or \
                 height != self.surface.get_height() or \
                 zoom != self._surface_zoom_level or \
-                start_ns < self._surface_start_ns or \
+                start_px < self._surface_start_px or \
                 rate != self._rate or \
-                end_ns > self._surface_end_ns:
+                end_px > self._surface_end_px:
+            # Generate a new surface since the previously generated one, if any,
+            # cannot be reused.
             if self.surface:
                 self.surface.finish()
                 self.surface = None
             self._surface_zoom_level = zoom
             # The generated waveform is for an extended range if possible,
             # so if the user scrolls we don't rebuild the waveform every time.
-            extra = self.pixel_to_ns(WAVEFORM_SURFACE_EXTRA_PX)
-            self._surface_start_ns = max(0, start_ns - extra)
-            self._surface_end_ns = min(end_ns + extra, max_duration)
+            self._surface_start_px = max(0, start_px - WAVEFORM_SURFACE_EXTRA_PX)
             self._rate = rate
+            self._surface_end_px = min(end_px + WAVEFORM_SURFACE_EXTRA_PX, max_duration_px)
 
             sample_duration = SAMPLE_DURATION / rate
-            range_start = min(max(0, int(self._surface_start_ns / sample_duration)), len(self.samples))
-            range_end = min(max(0, int(self._surface_end_ns / sample_duration)), len(self.samples))
+            range_start = min(max(0, int(self.pixel_to_ns(self._surface_start_px) / sample_duration)), 
len(self.samples))
+            range_end = min(max(0, int(self.pixel_to_ns(self._surface_end_px) / sample_duration)), 
len(self.samples))
             samples = self.samples[range_start:range_end]
-            surface_width = self.ns_to_pixel(self._surface_end_ns - self._surface_start_ns)
+            surface_width = self._surface_end_px - self._surface_start_px
             self.surface = renderer.fill_surface(samples, surface_width, height)
 
         # Paint the surface, ignoring the clipped rect.
@@ -1327,8 +1334,8 @@ class AudioPreviewer(Gtk.Layout, Previewer, Zoomable, Loggable):
         # the surface in context, if the entire asset would be drawn.
         # 2. - inpoint, because we're drawing a clip, not the entire asset.
         context.set_operator(cairo.OPERATOR_OVER)
-        offset = self.ns_to_pixel(self._surface_start_ns - inpoint)
-        context.set_source_surface(self.surface, offset, CLIP_BORDER_WIDTH)
+        offset_px = self._surface_start_px - inpoint_px
+        context.set_source_surface(self.surface, offset_px, CLIP_BORDER_WIDTH)
         context.paint()
 
     def _emit_done_on_idle(self):
diff --git a/pitivi/timeline/timeline.py b/pitivi/timeline/timeline.py
index 5dcfca382..111977038 100644
--- a/pitivi/timeline/timeline.py
+++ b/pitivi/timeline/timeline.py
@@ -1555,6 +1555,7 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
                     duration = self.timeline.mod_duration(original_duration)
                     if duration != original_duration:
                         obj.set_duration(duration)
+                        obj.props.max_duration = min(obj.props.max_duration, duration)
                 elif isinstance(obj, GES.Asset):
                     ges_clip = self.timeline.add_clip_to_layer(layer, obj, clip_position)
                     duration = ges_clip.props.duration
diff --git a/pitivi/utils/timeline.py b/pitivi/utils/timeline.py
index f87980658..36486d1c8 100644
--- a/pitivi/utils/timeline.py
+++ b/pitivi/utils/timeline.py
@@ -257,7 +257,7 @@ class EditingContext(GObject.Object, Loggable):
         app (Pitivi): The app.
     """
 
-    def __init__(self, focus, timeline, mode, edge, app, log_actions):
+    def __init__(self, focus, timeline, mode, edge, app, log_actions=None):
         GObject.Object.__init__(self)
         Loggable.__init__(self)
         if isinstance(focus, GES.TrackElement):
@@ -456,6 +456,7 @@ class Zoomable:
         # DIE YOU CUNTMUNCH CLOCK_TIME_NONE UBER STUPIDITY OF CRACK BINDINGS !!
         if duration == Gst.CLOCK_TIME_NONE:
             return 0
+
         return int((float(duration) / Gst.SECOND) * cls.zoomratio)
 
     @classmethod
@@ -465,6 +466,7 @@ class Zoomable:
         # DIE YOU CUNTMUNCH CLOCK_TIME_NONE UBER STUPIDITY OF CRACK BINDINGS !!
         if duration == Gst.CLOCK_TIME_NONE:
             return 0
+
         return (float(duration) / Gst.SECOND) * cls.zoomratio
 
     def zoom_changed(self):
diff --git a/tests/common.py b/tests/common.py
index f6168ea69..d074fc703 100644
--- a/tests/common.py
+++ b/tests/common.py
@@ -282,13 +282,12 @@ def setup_timeline(func):
         self.layer = self.timeline.append_layer()
         self.action_log = self.app.action_log
 
-        project = self.app.project_manager.current_project
         self.timeline_container = TimelineContainer(self.app, editor_state=self.app.gui.editor.editor_state)
-        self.timeline_container.set_project(project)
+        self.timeline_container.set_project(self.project)
         self.app.gui.editor.timeline_ui = self.timeline_container
 
         timeline = self.timeline_container.timeline
-        timeline.app.project_manager.current_project = project
+        self.assertEqual(timeline.app.project_manager.current_project, self.project)
         timeline.get_parent = mock.MagicMock(return_value=self.timeline_container)
 
         func(self)
@@ -541,6 +540,12 @@ class TestCase(unittest.TestCase, Loggable):
                         return element
         return None
 
+    def get_clip_element(self, ges_clip, element_class=GES.VideoSource):
+        for element in ges_clip.get_children(False):
+            if isinstance(element, element_class):
+                return element
+        return None
+
     @staticmethod
     def commit_cb(action_log, stack, stacks):
         stacks.append(stack)
diff --git a/tests/test_medialibrary.py b/tests/test_medialibrary.py
index cd4d4b11a..c65852215 100644
--- a/tests/test_medialibrary.py
+++ b/tests/test_medialibrary.py
@@ -110,7 +110,7 @@ class BaseTestMediaLibrary(common.TestCase):
             "notify::fraction", self._progress_bar_cb)
 
         self._create_assets(samples)
-        self.mainloop.run(timeout_seconds=20)
+        self.mainloop.run(timeout_seconds=25)
         self.assertFalse(self.medialibrary._progressbar.props.visible)
 
     def check_add_proxy(self, asset, scaled=False, w=160, h=120,
diff --git a/tests/test_previewers.py b/tests/test_previewers.py
index 2f0dfcf35..bdf320ade 100644
--- a/tests/test_previewers.py
+++ b/tests/test_previewers.py
@@ -15,12 +15,13 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this program; if not, see <http://www.gnu.org/licenses/>.
 """Tests for the timeline.previewers module."""
-# pylint: disable=protected-access
+# pylint: disable=protected-access,unused-argument
 import os
 import tempfile
 from unittest import mock
 
 import numpy
+from gi.repository import Gdk
 from gi.repository import GdkPixbuf
 from gi.repository import GES
 from gi.repository import Gst
@@ -31,6 +32,8 @@ from pitivi.timeline.previewers import Previewer
 from pitivi.timeline.previewers import THUMB_HEIGHT
 from pitivi.timeline.previewers import THUMB_PERIOD
 from pitivi.timeline.previewers import ThumbnailCache
+from pitivi.utils.timeline import EditingContext
+from pitivi.utils.timeline import Zoomable
 from tests import common
 from tests.test_medialibrary import BaseTestMediaLibrary
 
@@ -96,6 +99,66 @@ class TestAudioPreviewer(BaseTestMediaLibrary):
 
         self.assertEqual(samples, SIMPSON_WAVFORM_VALUES)
 
+    @common.setup_timeline
+    def test_offset(self):
+        # Set the videorate of tears_of_steel.webm.
+        self.project.videorate = Gst.Fraction(24, 1)
+        Zoomable.set_zoom_level(78)
+
+        timeline = self.timeline_container.timeline
+
+        start_frame = 43
+        start = self.timeline.get_frame_time(start_frame)
+        self.assertEqual(start, 1791666667)
+
+        clips = []
+        starts = []
+        asset = GES.UriClipAsset.request_sync(common.get_sample_uri("tears_of_steel.webm"))
+        for delta_frame in range(10):
+            ges_layer = timeline.ges_timeline.append_layer()
+            ges_clip = timeline.add_clip_to_layer(ges_layer, asset, start)
+
+            editing_context = EditingContext(ges_clip, self.timeline, GES.EditMode.EDIT_TRIM, 
GES.Edge.EDGE_START, self.app)
+            new_start = self.timeline.get_frame_time(start_frame + delta_frame)
+            editing_context.edit_to(new_start, ges_layer)
+            editing_context.finish()
+
+            clips.append(ges_clip)
+            starts.append(new_start)
+
+        # Check the clips.
+        expected_starts = [1791666667, 1833333334, 1875000000, 1916666667, 1958333334, 2000000000, 
2041666667, 2083333334, 2125000000, 2166666667]
+        self.assertListEqual(starts, expected_starts,
+                             "the start values calculated in the test are off")
+        self.assertListEqual([c.start for c in clips], expected_starts,
+                             "the start of the clips are wrong")
+        expected_inpoints = [0, 41666667, 83333333, 125000000, 166666667, 208333333, 250000000, 291666667, 
333333333, 375000000]
+        self.assertListEqual([c.inpoint for c in clips], expected_inpoints)
+
+        # Check the audio previewers.
+        audio_previewers = list(self.get_clip_element(c, GES.AudioSource).ui.previewer
+                                for c in clips)
+
+        offsets = []
+
+        def set_source_surface(surface, offset_x, offset_y):
+            offsets.append(offset_x)
+
+        samples = list(range(199))
+        for previewer in audio_previewers:
+            previewer.samples = samples
+            with mock.patch.object(Gdk, "cairo_get_clip_rectangle") as cairo_get_clip_rectangle:
+                cairo_get_clip_rectangle.return_value = (True, mock.Mock(x=0, width=10000))
+                from pitivi.timeline import previewers
+                with mock.patch.object(previewers.renderer, "fill_surface") as fill_surface:
+                    context = mock.Mock()
+                    context.set_source_surface = set_source_surface
+                    previewer.do_draw(context)
+            fill_surface.assert_called_once_with(samples, 949, -1)
+
+        expected_offsets = [0, -20, -40, -59, -79, -99, -119, -138, -158, -178]
+        self.assertListEqual(offsets, expected_offsets)
+
 
 class TestPreviewer(common.TestCase):
     """Tests for the `Previewer` class."""
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]