Skip to content

Commit 626ad8e

Browse files
authored
Fix Push AV Stream Verification player reading stale field names (#103) (#104)
* Fix Push AV Stream Verification player reading stale field names The Push AV Server's /streams API returns each stream's uploaded files under valid_uploads/error_uploads (lists of {file_path, reasons?}) since the server became session-oriented. The CLI's push_av_stream_verification.html was never updated and still looked for files/valid_files/invalid_files, which no longer exist in the response. As a result, allFiles was always empty, no .mpd/.m4s entry point was ever found, and the video player stayed blank even when the DUT successfully uploaded CMAF content to the server. Add getStreamFilePaths() to read valid_uploads/error_uploads first, falling back to the legacy files/valid_files/invalid_files shape for compatibility with older server responses. Also surface per-file non-conforming reasons in the Non-Conforming Files section using the reasons field now provided by error_uploads entries. Add regression tests asserting the rendered template references the current field names ahead of the legacy fallback. * Apply defensive null-checks to Push AV upload parsing per code review Use optional chaining (u?.file_path, u?.reasons) and filter(Boolean) when mapping valid_uploads/error_uploads entries to file paths and reasons, so a malformed or null entry in the server response can't throw a TypeError and block the verification page from rendering. Update the corresponding test assertion to check for the file_path field name generically instead of the literal 'u.file_path' loop variable expression, which no longer appears verbatim once optional chaining is used.
1 parent 16d3ddf commit 626ad8e

2 files changed

Lines changed: 82 additions & 6 deletions

File tree

tests/test_run/camera/test_camera_http_server.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,52 @@ def test_do_post_unknown_path_sends_404(self):
293293
assert handler._error_code == 404
294294

295295

296+
# ---------------------------------------------------------------------------
297+
# VideoStreamingHandler.serve_player - Push AV Stream Verification template
298+
# ---------------------------------------------------------------------------
299+
300+
301+
@pytest.mark.unit
302+
class TestServePlayerPushAVTemplate:
303+
"""Regression tests for issue #1051: the Push AV Server returns each stream's
304+
uploaded files under valid_uploads/error_uploads (list of {file_path, reasons?}),
305+
not the legacy files/valid_files/invalid_files shape. The rendered template's
306+
JS must read the current field names or the video player stays blank even
307+
when the DUT has successfully uploaded content."""
308+
309+
def _render(self):
310+
handler = _make_handler(
311+
path="/",
312+
server_attrs={
313+
"prompt_options": {"PASS": 1, "FAIL": 2},
314+
"prompt_text": "Verify the video stream",
315+
"is_push_av_verification": True,
316+
"push_av_server_url": "https://192.168.0.53:1234",
317+
},
318+
)
319+
handler.serve_player()
320+
return handler.wfile.getvalue().decode("utf-8")
321+
322+
def test_renders_without_template_error(self):
323+
html_content = self._render()
324+
assert "Template error" not in html_content
325+
assert "<!DOCTYPE html>" in html_content or "<html>" in html_content
326+
327+
def test_reads_valid_and_error_uploads_fields(self):
328+
html_content = self._render()
329+
assert "stream.valid_uploads" in html_content
330+
assert "stream.error_uploads" in html_content
331+
assert "file_path" in html_content
332+
333+
def test_no_longer_relies_solely_on_legacy_file_fields(self):
334+
"""The old field names may still appear as a fallback, but the current
335+
server field names must be checked first."""
336+
html_content = self._render()
337+
valid_uploads_idx = html_content.index("stream.valid_uploads")
338+
valid_files_idx = html_content.index("stream.valid_files")
339+
assert valid_uploads_idx < valid_files_idx
340+
341+
296342
# ---------------------------------------------------------------------------
297343
# VideoStreamingHandler.handle_response
298344
# ---------------------------------------------------------------------------

th_cli/test_run/camera/push_av_stream_verification.html

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,19 @@
526526
nonConformingSection.style.display = 'none';
527527
}}
528528

529+
// Push AV Server returns each stream's uploaded files as valid_uploads/error_uploads,
530+
// each entry being an object with a file_path (and reasons, for error_uploads).
531+
// Older server responses used plain filename arrays under files/valid_files/invalid_files -
532+
// keep supporting those too so this page degrades gracefully against either shape.
533+
function getStreamFilePaths(stream) {{
534+
if (stream.valid_uploads || stream.error_uploads) {{
535+
const validPaths = (stream.valid_uploads || []).map(u => u?.file_path).filter(Boolean);
536+
const invalidPaths = (stream.error_uploads || []).map(u => u?.file_path).filter(Boolean);
537+
return [...validPaths, ...invalidPaths];
538+
}}
539+
return stream.files || [...(stream.valid_files || []), ...(stream.invalid_files || [])];
540+
}}
541+
529542
function selectStream(index) {{
530543
if (!streamsData || !streamsData[index]) {{
531544
console.error('Stream data not found for index:', index);
@@ -535,8 +548,7 @@
535548
const stream = streamsData[index];
536549
selectedStreamId = stream.id || index;
537550

538-
// Combine valid_files and invalid_files (or use files if present)
539-
const allFiles = stream.files || [...(stream.valid_files || []), ...(stream.invalid_files || [])];
551+
const allFiles = getStreamFilePaths(stream);
540552

541553
// Update selected styling
542554
document.querySelectorAll('.stream-item').forEach(item => {{
@@ -582,9 +594,14 @@
582594
const streamNum = parseInt(stream.id) + 1;
583595
streamContentsTitle.textContent = `Stream ${{streamNum}} Contents`;
584596

585-
// Combine valid and invalid files (or use files if present)
586-
const validFiles = stream.valid_files || [];
587-
const invalidFiles = stream.invalid_files || [];
597+
const hasUploadsShape = !!(stream.valid_uploads || stream.error_uploads);
598+
const validFiles = hasUploadsShape
599+
? (stream.valid_uploads || []).map(u => u?.file_path).filter(Boolean)
600+
: (stream.valid_files || []);
601+
const invalidUploads = hasUploadsShape ? (stream.error_uploads || []) : [];
602+
const invalidFiles = hasUploadsShape
603+
? invalidUploads.map(u => u?.file_path).filter(Boolean)
604+
: (stream.invalid_files || []);
588605
const allFiles = stream.files || [...validFiles, ...invalidFiles];
589606

590607
// Display files
@@ -611,13 +628,26 @@
611628

612629
// Display conformance status
613630
if (invalidFiles.length > 0) {{
614-
nonConformingContent.innerHTML = `<div style="color: #c62828; text-align: center;">Found ${{invalidFiles.length}} non-conforming file(s)</div>`;
631+
const reasons = invalidUploads
632+
.flatMap(u => u?.reasons || [])
633+
.filter(Boolean);
634+
const reasonsHtml = reasons.length > 0
635+
? `<br><small>${{reasons.map(r => html_escape(r)).join('<br>')}}</small>`
636+
: '';
637+
nonConformingContent.innerHTML =
638+
`<div style="color: #c62828; text-align: center;">Found ${{invalidFiles.length}} non-conforming file(s)${{reasonsHtml}}</div>`;
615639
}} else {{
616640
nonConformingContent.innerHTML = '<div class="conforming-message">All files conform to Matter Spec.</div>';
617641
}}
618642
nonConformingSection.style.display = 'block';
619643
}}
620644

645+
function html_escape(str) {{
646+
const div = document.createElement('div');
647+
div.textContent = str;
648+
return div.innerHTML;
649+
}}
650+
621651
let currentDashPlayer = null; // Track current player instance
622652

623653
function playStream(streamUrl) {{

0 commit comments

Comments
 (0)