Skip to content

Make the audio player actually render - #191

Merged
mukunku merged 1 commit into
mukunku:mainfrom
PerikiyoXD:pr/audio-player-render
Aug 7, 2026
Merged

Make the audio player actually render#191
mukunku merged 1 commit into
mukunku:mainfrom
PerikiyoXD:pr/audio-player-render

Conversation

@PerikiyoXD

Copy link
Copy Markdown
Contributor

Audio column detection works, but the player never appears.
The cells show the raw byte string instead, so the feature is unreachable.

OnDataBindingComplete assigns column.CellTemplate = new AudioPlayerDataGridViewCell(),
and a DataGridView only consults the template when it creates a cell.

Data binding has already created every cell in the column by the time that event fires,
so the assignment has no effect on anything on screen.

Replacing the existing cells alongside the template is what makes the column switch over.

Traced through the detection path to confirm:

isAudioColumn=True                          <- detection is fine
cellTemplate=DataGridViewTextBoxCell        <- before the assignment
first cell is now DataGridViewTextBoxCell   <- after the assignment

Verified by hand afterwards on a file with wav columns: playback, pause, stop, seeking and the save
menu all work.

The rest of the 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.
    They are now captured before dispatching.
  • The ContextMenuStrip built on each right click was never disposed.

Also drops a duplicated null check, an unused local, and a field that was always null.

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.
@mukunku

mukunku commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Hello @PerikiyoXD ,

Thanks for your contribution. Before I can consider merging this PR, could you share a sample file and/or screenshot demonstrating the issue? When I open files with audio data in them the cells are rendered in the new template without needing to instantiate each cell:

audio-player

@PerikiyoXD

Copy link
Copy Markdown
Contributor Author

Yeah, so my idea was:
imagen

AUDIO_TEST.txt
(Rename to .parquet)

@PerikiyoXD

Copy link
Copy Markdown
Contributor Author

Latest shows this:
imagen

@PerikiyoXD

Copy link
Copy Markdown
Contributor Author

You can feel free to try my fork branches and reproduce it yourself

@mukunku

mukunku commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Thanks @PerikiyoXD ! Really appreciate the changes you made in your PR and for fixing this issue 🙌🏾 I'm going ahead and merging your fix in. I'll include it in the next release.

After taking a look it seems if the parent form of the grid is not rendered/visible yet, the cell template takes effect once the form + grid are shown via cell re-creation. So the audio player works in the Quick Peek Form. But if the form and grid are already visible, as you pointed out, the cells aren't re-created against the new cell template.

I appreciate the cleanup as well and marking the bool flag as volatile

@mukunku
mukunku merged commit 5f92117 into mukunku:main Aug 7, 2026
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants