Repository navigation
fix(preview): draw the processed frame at the encode's aspect (#149) - #153
Merged
Merged
Conversation
StuartCameronCode
marked this pull request as ready for review
November 10, 2026 07:14
…ave (#149) The comparison drew both sides at the source's display aspect, so any pass that reshaped the frame (resize, borders, crop, rotate, square pixels, a forced DAR) previewed distorted. A 720x576 PAL 4:3 capture resized to 1440x1080 and bordered out to 1920x1080 came out squashed into 4:3. - Worker: ProcessingPipeline::output_aspect is the single aspect decision, used by the encode's setsar/setdar and reported by the preview on its PREVIEW_INFO line as sar=N/D or dar=N/D. - App: PreviewFrameInfo parses it. The processed aspect is stored with the processed image on the QueueItem. BeforeAfterComparisonWidget takes a separate before/after aspect, and 1:1 is measured against the processed one. - The preview job now carries inputSar (via the shared FieldOrderDetector.normalizeSar). Without it {{SOURCE_SAR}} fell back to 1 and square-pixel fitting previewed a 1350-wide picture where the encode makes 1440. Tests: worker unit tests, test_185, preview_frame_info_test, preview_aspect_test, before_after_comparison_test, and a heavy preview group in integration_aspect_ratio_test comparing the preview's picture extent with the encode's.
… previews - Move the comparison's per-side aspect choice into MainViewModel (sourceDisplayAspect, comparisonAfterAspect) and the preview-arrival storage into _acceptProcessedPreview, so both are testable without a worker. - preview_aspect_test: an anamorphic source only denoised keeps 16:9 on both sides; the processed aspect survives a scrubber move (fails if computed from previewFrameInfo); falls back to the source's before a processed frame; cleared with a failed preview. - integration_aspect_ratio_test (heavy): denoise-only 16:11 source and a rotated 16:15 source, preview aspect against the encode's.
StuartCameronCode
force-pushed
the
agentbob/7-preview-aspect
branch
from
November 10, 2026 09:31
13c2bee to
6cc1d5b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #149.
Summary
The preview drew the processed frame at the source's display aspect. The preview PNG carries no aspect, so anything a pass reshaped came out squashed: for example, a 720x576 4:3 source resized and padded to 1920x1080. A second bug sat behind the first. The preview job never sent
inputSar, so{{SOURCE_SAR}}fell back to 1. Square-pixel fitting then previewed a different frame from the one encoded (1350 wide vs 1440).Worker
ProcessingPipeline::output_aspect(input_sar)(rotation-adjusted SAR →aspect_declaration). It is now the single decision behind both the encode'ssetsar/setdarand the preview.PREVIEW_INFO:line assar=N/Dordar=N/D, and prints nothing for square pixels.App
PreviewFrameInfo.displayAspectForturns the reported aspect plus the PNG's size into the shape to draw. A missing or malformed token falls back to the image's own pixel shape.BeforeAfterComparisonWidgettakes separatebeforeAspectRatioandafterAspectRatiovalues. 1:1 zoom is measured against the side being inspected.QueueItem(processedPreviewAspect) next to the image. If it were read frompreviewFrameInfo, it would be lost as soon as the scrubber moves (that field goes null then).sourceDisplayAspect,comparisonAfterAspect).PreviewGeneratorsendsinputSar, normalised through the sameFieldOrderDetector.normalizeSaras the encode job.Testing
cargo test(lib, filter_integration incl. newtest_185, preview_integration)cargo testsubtitle_integrationflutter test --exclude-tags heavyflutter test integration_aspect_ratio_test.dart(heavy)New heavy tests in the
preview aspect (issue #149)group check the preview against the actual encode in five cases: square, preserve, forced DAR, a denoise-only anamorphic source, and a rotated anamorphic source.Both changes were checked by breaking them on purpose:
encode 1440, preview 1350).previewFrameInfoinstead ofQueueItem, the scrubber-move unit test fails.For reviewers
pipeline_executor.rs: the encode'ssetsar/setdarnow goes throughoutput_aspect. Its behaviour should be identical (same two calls behind one helper).main_viewmodel.dart_acceptProcessedPreview: this is the only placeprocessedPreviewis assigned, and it now sets the aspect alongside it. A failed preview clears both.preservemode, a resize-and-pad to 1920x1080 re-stamps the source SAR (16:15), giving DAR 256:135 rather than 16:9. That is the existing Minor feature update suggestions #50 behaviour ofaspect_declaration, and the preview now shows it faithfully. Whetherpreserveshould re-stamp the SAR after resizing to a square-pixel target is a separate product question.QueueItem.copyWith(processedPreviewAspect: null)can't clear the field. That is consistent with the other nullable fields there, and nothing relies on it.