[GStreamer] Trim leading zeroes from track IDs #23668

vivienne-w · 2024-02-01T14:39:01Z

`48efea4`

[GStreamer] Trim leading zeroes from track IDs
https://bugs.webkit.org/show_bug.cgi?id=268549

Reviewed by Xabier Rodriguez-Calvar and Philippe Normand.

Qtdemux inserts leading zeroes when generating stream-ids, this patch
trims them in TrackPrivateBaseGStreamer for a better representation of
their underlying integers.

* Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp:
(WebCore::trimStreamId):
(WebCore::TrackPrivateBaseGStreamer::TrackPrivateBaseGStreamer):
(WebCore::TrackPrivateBaseGStreamer::notifyTrackOfStreamChanged):
(WebCore::TrackPrivateBaseGStreamer::trackIdFromPadStreamStartOrUniqueID):

Canonical link: https://commits.webkit.org/274469@main

3be515d

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 wincairo~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-02-01T14:39:09Z

EWS run on previous version of this PR (hash b334b04)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

eocanha · 2024-02-06T11:55:40Z

Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp

@@ -290,8 +301,7 @@ void TrackPrivateBaseGStreamer::notifyTrackOfStreamChanged()
    if (!streamId)
        return;

-    GST_INFO("Track %d got stream start for stream %s.", m_index, streamId.get());


Please, if you have the opportunity before merging the PR, add again this GST_INFO() line to the final patch. I find these log lines useful when having to debug track issues.
In order to do that, since the PR is already approved, you can just include this line in the commit message. No need to ask for an additional review:

Reviewed by Xabier Rodriguez-Calvar and Philippe Normand.

Thanks!

That line should be included automatically by the commit queue.

Even after resubmitting a patch with fixes? I thought any r+ granted was reset in that case. That's why I've always filled in the "Reviewed by" line when resubmitting last minute fixes after the patch was approved. Thanks for clarifying!

That happened before github, since github it works.

webkit-early-warning-system · 2024-02-12T16:33:47Z

EWS run on current version of this PR (hash 3be515d)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 wincairo~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

https://bugs.webkit.org/show_bug.cgi?id=268549 Reviewed by Xabier Rodriguez-Calvar and Philippe Normand. Qtdemux inserts leading zeroes when generating stream-ids, this patch trims them in TrackPrivateBaseGStreamer for a better representation of their underlying integers. * Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp: (WebCore::trimStreamId): (WebCore::TrackPrivateBaseGStreamer::TrackPrivateBaseGStreamer): (WebCore::TrackPrivateBaseGStreamer::notifyTrackOfStreamChanged): (WebCore::TrackPrivateBaseGStreamer::trackIdFromPadStreamStartOrUniqueID): Canonical link: https://commits.webkit.org/274469@main

webkit-commit-queue · 2024-02-12T18:33:58Z

Committed 274469@main (48efea4): https://commits.webkit.org/274469@main

Reviewed commits have been landed. Closing PR #23668 and removing active labels.

https://bugs.webkit.org/show_bug.cgi?id=270100 Reviewed by Xabier Rodriguez-Calvar. Use StringView::find() to trim zeroes instead of ::trim(), to not remove trailing zeroes. See previous PR: WebKit#23668 * Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp: (WebCore::trimStreamId): fix implementation Canonical link: https://commits.webkit.org/275378@main

https://bugs.webkit.org/show_bug.cgi?id=270100 Reviewed by Xabier Rodriguez-Calvar. Use StringView::find() to trim zeroes instead of ::trim(), to not remove trailing zeroes. See previous PR: WebKit/WebKit#23668 * Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp: (WebCore::trimStreamId): fix implementation Canonical link: https://commits.webkit.org/275378@main

…gi?id=270100 [GStreamer] Fix trimming of track IDs https://bugs.webkit.org/show_bug.cgi?id=270100 Reviewed by Xabier Rodriguez-Calvar. Use StringView::find() to trim zeroes instead of ::trim(), to not remove trailing zeroes. See previous PR: #23668 * Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp: (WebCore::trimStreamId): fix implementation Canonical link: https://commits.webkit.org/275378@main Canonical link: https://commits.webkit.org/274313.189@webkitglib/2.44

https://bugs.webkit.org/show_bug.cgi?id=270100 Reviewed by Xabier Rodriguez-Calvar. Use StringView::find() to trim zeroes instead of ::trim(), to not remove trailing zeroes. See previous PR: WebKit/WebKit#23668 * Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp: (WebCore::trimStreamId): fix implementation Canonical link: https://commits.webkit.org/275378@main

vivienne-w requested a review from philn as a code owner February 1, 2024 14:39

philn approved these changes Feb 1, 2024

View reviewed changes

calvaris approved these changes Feb 2, 2024

View reviewed changes

Ahmad-S792 assigned vivienne-w Feb 3, 2024

Ahmad-S792 added the WebKitGTK Bugs related to the Gtk API layer. label Feb 3, 2024

eocanha reviewed Feb 6, 2024

View reviewed changes

vivienne-w force-pushed the eng/GStreamer-Track-IDs-contain-unneccessary-leading-zeroes branch from b334b04 to 3be515d Compare February 12, 2024 16:33

philn added the merge-queue Applied to send a pull request to merge-queue label Feb 12, 2024

webkit-commit-queue force-pushed the eng/GStreamer-Track-IDs-contain-unneccessary-leading-zeroes branch from 3be515d to 48efea4 Compare February 12, 2024 18:33

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 12, 2024

webkit-commit-queue merged commit 48efea4 into WebKit:main Feb 12, 2024

vivienne-w mentioned this pull request Feb 26, 2024

[GStreamer] Fix trimming of track IDs #25114

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GStreamer] Trim leading zeroes from track IDs #23668

[GStreamer] Trim leading zeroes from track IDs #23668

vivienne-w commented Feb 1, 2024 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Feb 1, 2024 •

edited

Loading

eocanha Feb 6, 2024

calvaris Feb 7, 2024

eocanha Feb 7, 2024 •

edited

Loading

calvaris Feb 8, 2024

webkit-early-warning-system commented Feb 12, 2024 •

edited

Loading

webkit-commit-queue commented Feb 12, 2024

[GStreamer] Trim leading zeroes from track IDs #23668

[GStreamer] Trim leading zeroes from track IDs #23668

Conversation

vivienne-w commented Feb 1, 2024 • edited by webkit-early-warning-system Loading

48efea4

webkit-early-warning-system commented Feb 1, 2024 • edited Loading

eocanha Feb 6, 2024

Choose a reason for hiding this comment

calvaris Feb 7, 2024

Choose a reason for hiding this comment

eocanha Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

calvaris Feb 8, 2024

Choose a reason for hiding this comment

webkit-early-warning-system commented Feb 12, 2024 • edited Loading

webkit-commit-queue commented Feb 12, 2024

vivienne-w commented Feb 1, 2024 •

edited by webkit-early-warning-system

Loading

`48efea4`

webkit-early-warning-system commented Feb 1, 2024 •

edited

Loading

eocanha Feb 7, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 12, 2024 •

edited

Loading