From ebf205e07cb9aa710e4589d4af970d4708af8cd6 Mon Sep 17 00:00:00 2001 From: PerikiyoXD Date: Sat, 1 Aug 2026 21:05:37 +0200 Subject: [PATCH] Make the audio player actually render The audio column detection worked, but the player never appeared: cells showed the raw byte string instead. OnDataBindingComplete assigned column.CellTemplate, and a DataGridView only consults the template when it creates a cell. Binding has already created every cell in the column by the time that event fires, so the assignment had no effect on anything on screen. Replacing the existing cells alongside the template is what makes the column switch over. Confirmed with a trace through the detection path, which reported the column as audio, then reported the first cell as still being a DataGridViewTextBoxCell immediately after the template was set. Verified by hand afterwards: playback, pause, stop, seeking and the save menu all work on a file with wav columns. The rest of this change is in the cell itself, and matters now that the type is actually instantiated. _isInitialized is written by the background initialization task after four other fields and read by the UI thread while painting, with nothing ordering those writes; it is now volatile, which is what the "this shouldn't happen if we're initialized" null check in Paint was compensating for. Value and ValueType were read inside the background task, reaching into the grid's data binding off the UI thread, so they are now captured before dispatching. The context menu built on each right click was never disposed. Also drops a duplicated null check, an unused local, and a field that was always null. --- .../Controls/AudioPlayerDataGridViewCell.cs | 41 ++++++++++++++----- src/ParquetViewer/Controls/ParquetGridView.cs | 8 ++++ 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/ParquetViewer/Controls/AudioPlayerDataGridViewCell.cs b/src/ParquetViewer/Controls/AudioPlayerDataGridViewCell.cs index cf36cad..cf74a0a 100644 --- a/src/ParquetViewer/Controls/AudioPlayerDataGridViewCell.cs +++ b/src/ParquetViewer/Controls/AudioPlayerDataGridViewCell.cs @@ -20,7 +20,11 @@ internal class AudioPlayerDataGridViewCell : DataGridViewTextBoxCell private Timer _updateTimer = new() { Interval = 100 }; private Timer _initializationTimer = new() { Interval = 100 }; - private bool _isInitialized = false; + //Written by the background initialization task and read by the UI thread while painting. + //volatile gives the release/acquire ordering that makes the writes above it visible: without it + //the UI thread could observe _isInitialized as true while _audioStream was still null, which is + //what the defensive null check in Paint is working around. + private volatile bool _isInitialized = false; private AudioFormat? _audioFormat = AudioFormat.Invalid; private string _errorMessage = "loading..."; private bool _isCellTooSmall = false; @@ -31,7 +35,6 @@ internal class AudioPlayerDataGridViewCell : DataGridViewTextBoxCell private Rectangle _trackBarBounds; private Rectangle _contextMenuButtonBounds; - private string? _debugText = null; private bool _isCursorHoveringPlayPauseButton; private bool _isCursorHoveringStopButton; private bool _isCursorHoveringMenuButton; @@ -56,13 +59,27 @@ public AudioPlayerDataGridViewCell() private void RedrawCell() => DataGridView?.InvalidateCell(this); private Task? _initializationTask = null; + + /// + /// Kicks off audio initialization on a background thread, once per cell. + /// + /// Must be called from the UI thread: it reads the cell's Value before dispatching. private Task InitializePlayerAsync() - => _initializationTask ??= Task.Run(() => + { + if (_initializationTask is not null) + return _initializationTask; + + //Read the cell state here rather than inside the task. Value and ValueType reach into the + //grid's data binding, which is not safe to touch from a background thread. + var cellValue = this.Value; + var cellValueTypeName = this.ValueType?.Name ?? "null"; + + return _initializationTask = Task.Run(() => { try { //Prepare audio stream - if (this.Value is IByteArrayValue byteArray) + if (cellValue is IByteArrayValue byteArray) { this._audioStream = GetAudioStream(byteArray.Data, out var audioFormat); this._audioFormat = audioFormat; @@ -73,13 +90,13 @@ private Task InitializePlayerAsync() this._audioPlayer.Init(this._audioStream); this._audioPlayer.PlaybackStopped += OnPlaybackStopped; } - else if (this.Value == DBNull.Value) + else if (cellValue == DBNull.Value) { this._audioFormat = null; } else { - throw new InvalidDataException($"{this.ValueType.Name} was not the expected type {nameof(IByteArrayValue)}"); + throw new InvalidDataException($"{cellValueTypeName} was not the expected type {nameof(IByteArrayValue)}"); } } catch (Exception ex) @@ -89,9 +106,11 @@ private Task InitializePlayerAsync() } finally { + //Set last: the volatile write publishes every field assigned above to the UI thread. this._isInitialized = true; } }); + } private void OnPlaybackStopped(object? source, StoppedEventArgs args) { @@ -202,15 +221,13 @@ protected override void Paint(Graphics graphics, Rectangle clipBounds, Rectangle timeFormat += this._audioStream.TotalTime.TotalSeconds < 0 ? @"\.fff" : string.Empty; //show milliseconds if the audio is less than 1 second string currentTime = this._audioStream.CurrentTime.ToString(timeFormat) ?? TimeSpan.FromSeconds(0).ToString(timeFormat); string totalTime = this._audioStream.TotalTime.ToString(timeFormat) ?? TimeSpan.FromSeconds(0).ToString(timeFormat); - TextRenderer.DrawText(graphics, _debugText ?? $"{currentTime} / {totalTime}", cellStyle.Font, _trackBarBounds, Theme.LightModeTheme.TextColor, TextFormatFlags.VerticalCenter | TextFormatFlags.HorizontalCenter); + TextRenderer.DrawText(graphics, $"{currentTime} / {totalTime}", cellStyle.Font, _trackBarBounds, Theme.LightModeTheme.TextColor, TextFormatFlags.VerticalCenter | TextFormatFlags.HorizontalCenter); } protected override void OnMouseMove(DataGridViewCellMouseEventArgs e) { base.OnMouseMove(e); - Rectangle translatedBounds = this._playPauseButtonBounds; - translatedBounds.Offset(-this._cellBounds.Location.X, -this._cellBounds.Location.Y); this._isCursorHoveringPlayPauseButton = ContainsCursor(this._playPauseButtonBounds, e.Location); this._isCursorHoveringStopButton = ContainsCursor(this._stopButtonBounds, e.Location); this._isCursorHoveringMenuButton = ContainsCursor(this._contextMenuButtonBounds, e.Location); @@ -322,7 +339,7 @@ private void TogglePlayPause() private void StopPlayback() { - if (this._audioPlayer == null || this._audioPlayer == null) + if (this._audioPlayer == null) return; if (this._audioPlayer.PlaybackState != PlaybackState.Stopped) @@ -411,6 +428,10 @@ private void ShowContextMenu(Point location) } } + //Show() is modeless, so the menu can't be disposed inline. Dispose it once it closes instead, + //otherwise every right click leaks a strip and its items for the lifetime of the grid. + menu.Closed += (_, _) => menu.BeginInvoke(menu.Dispose); + menu.Show(this.DataGridView, location + (Size)this._cellBounds.Location); menu.PerformLayout(); diff --git a/src/ParquetViewer/Controls/ParquetGridView.cs b/src/ParquetViewer/Controls/ParquetGridView.cs index 66bd266..ebf9646 100644 --- a/src/ParquetViewer/Controls/ParquetGridView.cs +++ b/src/ParquetViewer/Controls/ParquetGridView.cs @@ -1440,6 +1440,14 @@ protected override void OnDataBindingComplete(DataGridViewBindingCompleteEventAr //exceptions "seem" to be innocuous so going to keep doing it this way for now. //Only other alternative is to stop using AutoGenerateColumns :/ column.CellTemplate = new AudioPlayerDataGridViewCell(); + + //Assigning the template only affects cells created from this point on, and binding + //has already created every cell in the column by the time this event fires. Without + //replacing them the column keeps its text box cells and renders the raw bytes. + foreach (DataGridViewRow row in this.Rows) + { + row.Cells[column.Index] = new AudioPlayerDataGridViewCell(); + } } } }