Change sample time line to vertical layout by pengbins · Pull Request #531 · gpac/mp4box.js

pengbins · 2025-12-07T02:55:02Z

Description

This PR try to imporve the readability of sample timeline in filereader demo. The horizontal layout for displaying Decoding and Composition mapping was too compact and difficult to interpret. The changes introduce a vertical layout and add hover highlighting to clearly visualize the correspondence between Decoding and Composition sample mappings.

Changes Made

Layout Restructuring

Changed from a compact horizontal arrangement to a clear vertical stacked layout
Improved visual separation and alignment between Decoding and Composition timelines

Interactive Enhancement

Added mouse hover highlighting that visually links corresponding Decoding and Composition pair.

Benefits

✅ Enhanced Readability: Vertical layout provides more space and reduces visual clutter

✅ Better Mapping Visualization: Hover highlighting instantly shows Decoding ↔ Composition relationships

✅ Improved User Experience: Intuitive interaction helps users understand sample mappings without guesswork

✅ Maintains All Functionality: All existing features remain fully functional

Visual Comparison

Before: Compact horizontal layout making it hard to trace mappings

After: Clean vertical layout with hover highlighting for clear relationship visualization

demo

rbouqueau · 2025-12-08T12:27:37Z

Looks better, thanks for your contribution. I'll let @DenizUgur validate the technical side and merge.

changeset-bot · 2026-05-24T20:40:35Z

⚠️ No Changeset found

Latest commit: cf45a9a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

rbouqueau · 2026-10-15T18:14:38Z

Didn't get any feedback so I submitted it roughly to my LLM:

I’d keep the vertical layout and hover approach, but fix the SVG sizing before merging. I reviewed [PR #531](#531) at 89fb972.

The main problem is that [the SVG viewport still uses the browser’s dimensions](

mp4box.js/demo/filereader.js

Lines 1077 to 1093 in 89fb972

    
           function SampleTimeline() { 
        
             var margin = { top: 10, right: 10, bottom: 80, left: 20 }; 
        
             var width = (this.width = document.body.clientWidth - margin.left - margin.right); 
        
             var height = (this.height = window.innerHeight - margin.top - margin.bottom); 
        
             var div = d3.select('#sampletimeline'); 
        
             div.html(''); 
        
             this.svg = div 
        
               .append('svg') 
        
               .attr('width', '100%') 
        
               .attr('height', '100%') 
        
               .attr( 
        
                 'viewBox', 
        
                 '0 0 ' + (width + margin.left + margin.right) + ' ' + (height + margin.top + margin.bottom), 
        
               ) 
        
               .append('g') 
        
               .attr('transform', 'translate(' + margin.left + ',' + margin.top + ')');

), while the vertical content grows with sample timestamps.

Using the actual D3 code, I reproduced two regressions:

1280×600, 20 equal-duration samples: the old layout fits everything; the PR places the final two sample pairs outside the viewport.
320-pixel width, two samples: the old layout fits; the PR’s composition column extends to x=380 and is partially clipped.

Two inherited issues also remain: negative composition timestamps can fall outside the viewport, and updating with an empty selection leaves the previous samples displayed.

I prepared a refinement patch, applicable on top of the PR. It:

Sizes the SVG to the full timestamp range, allowing normal page scrolling.
Uses a content-sized width that scales down on narrow screens.
Includes both DTS and CTS when choosing the timeline origin.
Clears old samples when the selection becomes empty.

Validation: ten targeted DOM/SVG scenarios passed, including reordered timestamps, variable durations, and repeated updates. JavaScript syntax checking passed. Browser visual and hover verification remain untested because browser access was unavailable.

Your checkout is unchanged.

The LLM suggested the following update:

--- a/demo/filereader.js
+++ b/demo/filereader.js
@@ -1006,18 +1006,23 @@
   }
 }
 
-function verticalTimeline () {
-  var that = this;
-  var data = this.data;
-  if (!data || data.length === 0) {
-    return;
-  }
-  var scale = data[0].duration ? 30 / data[0].duration : 30;
+function verticalTimeline() {
+  var data = this.data || [];
+  var scale = data.length && data[0].duration ? 30 / data[0].duration : 30;
   const sample_width = 120;
-  const content =   this.svg.select("g.content");
+  const content = this.svg.select('g.content');
   content.html('');
-  var dts_offset = data[0].dts;
+  var dts_offset = d3.min(data, function (d) {
+    return Math.min(d.dts, d.cts);
+  }) || 0;
+  var timeline_end = d3.max(data, function (d) {
+    return Math.max(d.dts, d.cts) + d.duration;
+  }) || 0;
   var ypos = 50;
+  this.height = ypos + (timeline_end - dts_offset) * scale;
+  var width = this.width + this.margin.left + this.margin.right;
+  var height = this.height + this.margin.top + this.margin.bottom;
+  this.viewport.attr('height', height).attr('viewBox', '0 0 ' + width + ' ' + height);
   data.forEach(function (d, i) {
     var sampleg = content.append('g').attr("class","pair");
     const dts_y = (d.dts - dts_offset) * scale + ypos;
@@ -1075,24 +1080,28 @@
 SampleTimeline.prototype.update = verticalTimeline;
 
 function SampleTimeline() {
-  var margin = { top: 10, right: 10, bottom: 80, left: 20 };
-  var width = (this.width = document.body.clientWidth - margin.left - margin.right);
-  var height = (this.height = window.innerHeight - margin.top - margin.bottom);
+  var margin = (this.margin = { top: 10, right: 10, bottom: 20, left: 20 });
+  const sample_height = 30;
+  const sample_width = 120;
+  var width = (this.width = 3 * sample_width);
+  var height = (this.height = 50);
 
   var div = d3.select('#sampletimeline');
   div.html('');
-  this.svg = div
+  this.viewport = div
     .append('svg')
-    .attr('width', '100%')
-    .attr('height', '100%')
+    .attr('width', width + margin.left + margin.right)
+    .attr('height', height + margin.top + margin.bottom)
+    .style('max-width', '100%')
+    .style('height', 'auto')
+    .style('display', 'block')
     .attr(
       'viewBox',
       '0 0 ' + (width + margin.left + margin.right) + ' ' + (height + margin.top + margin.bottom),
-    )
+    );
+  this.svg = this.viewport
     .append('g')
     .attr('transform', 'translate(' + margin.left + ',' + margin.top + ')');
-  const sample_height = 30;
-  const sample_width = 120;
   this.svg
     .append('text')
     .attr('x', sample_width/2)

Do you agree with it?

pengbins · 2026-10-16T03:36:43Z

Didn't get any feedback so I submitted it roughly to my LLM:

I’d keep the vertical layout and hover approach, but fix the SVG sizing before merging. I reviewed [PR #531](#531) at 89fb972.

The main problem is that [the SVG viewport still uses the browser’s dimensions](

mp4box.js/demo/filereader.js

Lines 1077 to 1093 in 89fb972

function SampleTimeline() {

var margin = { top: 10, right: 10, bottom: 80, left: 20 };

var width = (this.width = document.body.clientWidth - margin.left - margin.right);

var height = (this.height = window.innerHeight - margin.top - margin.bottom);

var div = d3.select('#sampletimeline');

div.html('');

this.svg = div

.append('svg')

.attr('width', '100%')

.attr('height', '100%')

.attr(

'viewBox',

'0 0 ' + (width + margin.left + margin.right) + ' ' + (height + margin.top + margin.bottom),

)

.append('g')

.attr('transform', 'translate(' + margin.left + ',' + margin.top + ')');

), while the vertical content grows with sample timestamps.
Using the actual D3 code, I reproduced two regressions:

1280×600, 20 equal-duration samples: the old layout fits everything; the PR places the final two sample pairs outside the viewport.

320-pixel width, two samples: the old layout fits; the PR’s composition column extends to x=380 and is partially clipped.

Two inherited issues also remain: negative composition timestamps can fall outside the viewport, and updating with an empty selection leaves the previous samples displayed.

I prepared a refinement patch, applicable on top of the PR. It:

Sizes the SVG to the full timestamp range, allowing normal page scrolling.

Uses a content-sized width that scales down on narrow screens.

Includes both DTS and CTS when choosing the timeline origin.

Clears old samples when the selection becomes empty.

Validation: ten targeted DOM/SVG scenarios passed, including reordered timestamps, variable durations, and repeated updates. JavaScript syntax checking passed. Browser visual and hover verification remain untested because browser access was unavailable.

Your checkout is unchanged.

The LLM suggested the following update:
--- a/demo/filereader.js
+++ b/demo/filereader.js
@@ -1006,18 +1006,23 @@
   }
 }
 
-function verticalTimeline () {
-  var that = this;
-  var data = this.data;
-  if (!data || data.length === 0) {
-    return;
-  }
-  var scale = data[0].duration ? 30 / data[0].duration : 30;
+function verticalTimeline() {
+  var data = this.data || [];
+  var scale = data.length && data[0].duration ? 30 / data[0].duration : 30;
   const sample_width = 120;
-  const content =   this.svg.select("g.content");
+  const content = this.svg.select('g.content');
   content.html('');
-  var dts_offset = data[0].dts;
+  var dts_offset = d3.min(data, function (d) {
+    return Math.min(d.dts, d.cts);
+  }) || 0;
+  var timeline_end = d3.max(data, function (d) {
+    return Math.max(d.dts, d.cts) + d.duration;
+  }) || 0;
   var ypos = 50;
+  this.height = ypos + (timeline_end - dts_offset) * scale;
+  var width = this.width + this.margin.left + this.margin.right;
+  var height = this.height + this.margin.top + this.margin.bottom;
+  this.viewport.attr('height', height).attr('viewBox', '0 0 ' + width + ' ' + height);
   data.forEach(function (d, i) {
     var sampleg = content.append('g').attr("class","pair");
     const dts_y = (d.dts - dts_offset) * scale + ypos;
@@ -1075,24 +1080,28 @@
 SampleTimeline.prototype.update = verticalTimeline;
 
 function SampleTimeline() {
-  var margin = { top: 10, right: 10, bottom: 80, left: 20 };
-  var width = (this.width = document.body.clientWidth - margin.left - margin.right);
-  var height = (this.height = window.innerHeight - margin.top - margin.bottom);
+  var margin = (this.margin = { top: 10, right: 10, bottom: 20, left: 20 });
+  const sample_height = 30;
+  const sample_width = 120;
+  var width = (this.width = 3 * sample_width);
+  var height = (this.height = 50);
 
   var div = d3.select('#sampletimeline');
   div.html('');
-  this.svg = div
+  this.viewport = div
     .append('svg')
-    .attr('width', '100%')
-    .attr('height', '100%')
+    .attr('width', width + margin.left + margin.right)
+    .attr('height', height + margin.top + margin.bottom)
+    .style('max-width', '100%')
+    .style('height', 'auto')
+    .style('display', 'block')
     .attr(
       'viewBox',
       '0 0 ' + (width + margin.left + margin.right) + ' ' + (height + margin.top + margin.bottom),
-    )
+    );
+  this.svg = this.viewport
     .append('g')
     .attr('transform', 'translate(' + margin.left + ',' + margin.top + ')');
-  const sample_height = 30;
-  const sample_width = 120;
   this.svg
     .append('text')
     .attr('x', sample_width/2)
Do you agree with it?

Thanks a lot. I have applied this update to this PR.

In the original horizontal layout, the SVG also can't include all samples when there are too many samples. There is no horizontal scroll bar. So I didn't try to update the SVG size when more samples are selected.

With your suggestion, it is much better that the selected samples are always visible.

Change sample time line to vertical layout

426f60a

pengbins force-pushed the sample_time_line_vertical_layout branch from 35328df to 426f60a Compare February 11, 2026 10:15

DenizUgur force-pushed the main branch from d0aed28 to 2af7e20 Compare May 24, 2026 20:27

Merge branch 'main' into sample_time_line_vertical_layout

89fb972

pengbins added 2 commits October 16, 2026 11:28

Fix the SVG sizing before merging

886730d

Merge branch 'gpac:main' into sample_time_line_vertical_layout

cf45a9a

Navigation Menu

Repository navigation

Change sample time line to vertical layout - #531

Change sample time line to vertical layout#531
pengbins wants to merge 4 commits into
gpac:mainfrom
pengbins:sample_time_line_vertical_layout

pengbins commented Dec 7, 2025 •

edited

Loading

Uh oh!

rbouqueau commented Dec 8, 2025

Uh oh!

changeset-bot Bot commented May 24, 2026 •

edited

Loading

Uh oh!

rbouqueau commented Oct 15, 2026

Uh oh!

pengbins commented Oct 16, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

pengbins commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes Made

Benefits

Visual Comparison

Uh oh!

rbouqueau commented Dec 8, 2025

Uh oh!

changeset-bot Bot commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

rbouqueau commented Oct 15, 2026

Uh oh!

pengbins commented Oct 16, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pengbins commented Dec 7, 2025 •

edited

Loading

changeset-bot Bot commented May 24, 2026 •

edited

Loading