Opened 10 years ago

Closed 8 years ago

#7517 closed Patch - Feature (Won't Fix)

Separate Volume control from Audio Output

Reported by: mythtv@… Owned by: JYA
Priority: minor Milestone: unknown
Component: MythTV - Audio Output Version: Master Head
Severity: low Keywords: Volume
Cc: Ticket locked: no

Description

The existing implementation for controlling audio volume from within MythTV is associated with the Audio Output system. The attached patches aim to separate the functionality of adjusting audio volume from the playback of audio content.

I feel that the proposed volume control implementation contained within the attached patches provides a number of improvements:

  • Volume Control Enumeration - The system provides user-friendly names within the Audio setup pages of MythFrontend.
  • Volume Control State Externalised - The system relies on the state of the volume control being stored externally. This approach simplifies scenarios such as changing channels whilst muted, which previously required special handling within the code.
  • Volume Control State Change Events - Any user interface component (e.g. the volume OSD) can be connected to the volume control interface so that it can be displayed whenever the volume control state changes. This approach correctly handles scenarios where the volume control may be modified by a means other than MythTV keypresses (e.g. if there are hardware controls for the volume). This follows more of a "model", "view", "controller" architecture.

With the exception of the software volume control implementation all interfaces target system level volume controls. Normally such controls should not be modified by an application, however as the primary intention of MythTV is to take control of the whole PC, these implementations provide the best results.

In order to provide device enumeration, these patches increase the minimum required version of OSS to be version 4 or greater. This makes it incompatible with OSS version 3, which is currently shipped with the linux kernel.

For correct operation, the patches attached to Ticket #6662 should also be applied.

Attachments (18)

volume_control_mythtv_r22764.patch (128.7 KB) - added by mythtv@… 10 years ago.
volume_control_mythplugins_r22764.patch (10.8 KB) - added by mythtv@… 10 years ago.
volume_WinSDK_includes.zip (50.0 KB) - added by mythtv@… 10 years ago.
WinSDK include files to be placed within the MinGW "/usr/include" directory
volume_control_mythtv_r22850.patch (128.0 KB) - added by mythtv@… 10 years ago.
volume_control_mythplugins_r22850.patch (10.8 KB) - added by mythtv@… 10 years ago.
volume_control_mythtv_r22928.patch (128.0 KB) - added by mythtv@… 10 years ago.
volume_control_mythplugins_r22928.patch (10.6 KB) - added by mythtv@… 10 years ago.
volume_control_mythtv_r23492.patch (76.5 KB) - added by mythtv@… 10 years ago.
volume_control_mythplugins_r23492.patch (10.6 KB) - added by mythtv@… 10 years ago.
volume_control_trunk_r24763.patch (139.3 KB) - added by mythtv@… 9 years ago.
volume_control_trunk_r24896.patch (126.2 KB) - added by mythtv@… 9 years ago.
volume_control_trunk_r25013.patch (129.4 KB) - added by mythtv@… 9 years ago.
volume_control_trunk_r25065.patch (130.4 KB) - added by mythtv@… 9 years ago.
volume_control_trunk_r25178.patch (130.5 KB) - added by mythtv@… 9 years ago.
volume_control_trunk_r25559.patch (133.6 KB) - added by mythtv@… 9 years ago.
volume_control_trunk_r25745.patch (134.9 KB) - added by mythtv@… 9 years ago.
volume_control_trunk_r25902.patch (131.8 KB) - added by mythtv@… 9 years ago.
volume_control_trunk_r26401.patch (132.0 KB) - added by mythtv@… 9 years ago.

Download all attachments as: .zip

Change History (26)

Changed 10 years ago by mythtv@…

Changed 10 years ago by mythtv@…

Changed 10 years ago by mythtv@…

Attachment: volume_WinSDK_includes.zip added

WinSDK include files to be placed within the MinGW "/usr/include" directory

Changed 10 years ago by mythtv@…

Changed 10 years ago by mythtv@…

Changed 10 years ago by mythtv@…

Changed 10 years ago by mythtv@…

Changed 10 years ago by mythtv@…

Changed 10 years ago by mythtv@…

Changed 9 years ago by mythtv@…

Changed 9 years ago by mythtv@…

Changed 9 years ago by mythtv@…

Changed 9 years ago by mythtv@…

Changed 9 years ago by mythtv@…

comment:1 Changed 9 years ago by JYA

Severity: mediumlow
Status: newinfoneeded_new

I'm not sure what you are trying to achieve here, or what problem you are trying to *fix*...

Moving volume control from within the sub AO class, only to have to retest externally which framework is to be used , IMHO only adds complexity , extra code, more files, all of those without solving anything...

Also, a requirement on OSS >= 4 is unlikely to ever be committed

comment:2 in reply to:  1 Changed 9 years ago by anonymous

Replying to jyavenard:

I'm not sure what you are trying to achieve here, or what problem you are trying to *fix*...

The outcomes that I wish to achieve with these modifications are to reduce code complexity and increase end user friendliness and experience.

The ultimate problem that I am wishing to fix is that of being unable to modify the volume when no audio is being played. The frequent problem of listening to music or video with the volume high, exiting, and then some time later listening to audio only to be blasted with the previous volume. Consumer electronics and other software applications rarely impose the restriction that it is only possible to change the volume when sound is being emitted.

Moving volume control from within the sub AO class, only to have to retest externally which framework is to be used , IMHO only adds complexity , extra code, more files, all of those without solving anything...

I appreciate the reasons behind the existing behaviour based upon the current implementation, which is why I am proposing a separation of concerns between the audio output objects and separate volume control objects. I hope that if you consider the volume control more in terms of a user interface component and less in terms of the affect it has on the audio then this separation of concerns may become clearer.

One aspect of the proposed implementation is that volume change notifications be event driven. This approach allows for further simplification of code by allowing multiple volume control objects to be instantiated all referencing the same underlying hardware. So, for example, a volume control object may be instantiated purely to access volume change events in order to drive the graphical aspects of presenting a volume level overlay; But it may be advantageous to instantiate a separate object within the key-press handling code that drives the adjustment of the volume. Such an approach elegantly follows the model-view-controller paradigm and is felt to be of particular importance in the ideal scenario where the volume is controlled “globally” from the main menu key-press handler but MythMusic and TV playback wish to present graphical feedback of the changes.

Also, a requirement on OSS >= 4 is unlikely to ever be committed

The requirement for OSS >= 4 stems from the desire to present user-friendly textual descriptions of audio hardware as opposed to the rather cryptic machine-friendly mechanisms used internally, as OSS 3 does not offer such a capability. My rationale is that if anyone wishes to use OSS over ALSA, then they are likely to already be using version 4. Personally, I feel that we should either support OSS 4 or not support OSS. The OSS 4 implementation is provided purely for completeness, I have no intention of ever using it.

I am happy to add a similar user-friendly enumeration implementation for audio output classes (although I notice that this is an area you have already made improvements to recently) and to make the modifications necessary to realise my goal of a “global” volume implementation. I have not developed such an implementation to date due to the relatively invasive nature of such a change and the challenges associated with maintaining such changes over a prolonged period of time. Simply removing the volume control functionality from existing audio output objects is the first step along this road and has been relatively easy to maintain. Although the number of updated patches attached to this ticket are testaments to the fact that it is not entirely trivial.

If there is any further information I may provide you with then please do not hesitate to ask. I appreciate the time you have spent in consideration of these modifications.

comment:3 Changed 9 years ago by stuartm

Status: infoneeded_newnew

Changed 9 years ago by mythtv@…

comment:4 Changed 9 years ago by robertm

Status: newassigned

Changed 9 years ago by mythtv@…

comment:5 Changed 9 years ago by JYA

Milestone: unknown0.25

I do plan to implement something that will provide similar end-result... But won't be for 0.24.

And I'm not a fan of your solution, way too much duplicated code

Changed 9 years ago by mythtv@…

Changed 9 years ago by mythtv@…

comment:6 Changed 9 years ago by stuartm

Milestone: 0.25

Milestone 0.25 deleted

comment:7 Changed 8 years ago by beirdo

Milestone: 0.25unknown

comment:8 Changed 8 years ago by stuartm

Resolution: Won't Fix
Status: assignedclosed
Type: enhancementPatch - Feature
Version: unknownMaster Head
Note: See TracTickets for help on using tickets.