home
products
contribute
download
documentation
forum
Home
Forums
New posts
Search forums
What's new
New posts
All posts
Latest activity
Members
Registered members
Current visitors
Donate
Log in
Register
What's new
Search
Search
Search titles only
By:
New posts
Search forums
Search titles only
By:
Menu
Log in
Register
Navigation
Install the app
Install
More options
Contact us
Close Menu
Forums
MediaPortal 1
Quality Assurance
Bugreports
Archive
ACTION_EJECTCD doesn't work
Contact us
RSS
JavaScript is disabled. For a better experience, please enable JavaScript in your browser before proceeding.
You are using an out of date browser. It may not display this or other websites correctly.
You should upgrade or use an
alternative browser
.
Reply to thread
Message
<blockquote data-quote="mm1352000" data-source="post: 1185040" data-attributes="member: 82144"><p>Thanks for testing guys, and for your detailed reports - much appreciated. <img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" class="smilie smilie--sprite smilie--sprite24" alt="(y)" title="Thumbs Up (y)" loading="lazy" data-shortname="(y)" /></p><p></p><p>Let me try to cover off all of the points you've raised. Apologies in advance for the length. Please bear with me.</p><p></p><p></p><p>That's fine.</p><p></p><p></p><p>Yeah, that DLL won't work with 1.12 - sorry. I knew choosing the right version to build would be a bit of a trick. For myself, I still have MP 1.7.1 so I currently can't test anything I build either.</p><p></p><p></p><p>I think this may well be a side-effect of the mismatching builds. 1.15 PR would be close enough to 1.14 in most cases, though some things may not work. The addition of Stéphane's changes throws an extra spanner in the works. I'll see if I can build a 1.12 version for your production system, and maybe throw in a 1.15 PR version as optional. I won't be able to include Stéphane's changes - too time consuming.</p><p></p><p></p><p>From the code, this is expected:</p><p><a href="https://github.com/MediaPortal/MediaPortal-1/blob/master/mediaportal/MediaPortal.Application/MediaPortal.cs#L4170" target="_blank">https://github.com/MediaPortal/MediaPortal-1/blob/master/mediaportal/MediaPortal.Application/MediaPortal.cs#L4170</a></p><p></p><p>Please be aware that it's difficult if not impossible to detect the difference between empty drive and open tray. Thus writing logic to <em>always </em>open and close the tray appropriately with only one button is tricky... and that's not even considering drives that don't even have a tray. It may be possible to implement some kind of improvement in future but for me and for now I'm strictly limiting scope to fixing bugs without causing any regressions.</p><p></p><p></p><p>I confirm that I can reproduce with MP 1.7.1. <sigh> <img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" class="smilie smilie--sprite smilie--sprite3" alt=":(" title="Frown :(" loading="lazy" data-shortname=":(" /></p><p>The relevant code:</p><p>[code]</p><p>case 654: // Eject</p><p> if (item != null && Util.Utils.getDriveType(item.Path) != 5)</p><p> {</p><p> Util.Utils.EjectCDROM();</p><p> }</p><p> else</p><p> {</p><p> if (item != null && item.Path != null)</p><p> {</p><p> var driveInfo = new DriveInfo(Path.GetPathRoot(item.Path));</p><p></p><p> if (!driveInfo.IsReady)</p><p> {</p><p> Util.Utils.CloseCDROM(Path.GetPathRoot(item.Path));</p><p> }</p><p> else</p><p> {</p><p> Util.Utils.EjectCDROM(Path.GetPathRoot(item.Path));</p><p> }</p><p> }</p><p> }</p><p> LoadDirectory(string.Empty);[/code]</p><p></p><p>First clause says if the drive is not a CD/DVD/BR drive (removable USB???), just eject [the first/default CD/DVD/BR drive]. To be honest this makes no sense to me, but what the hey. Maybe's it's catch-all code.</p><p>The second clause is the relevant clause. It says: if a disc is not loaded in the drive (ie. tray is open <strong>OR</strong> tray is closed with no disc), close the tray... but if a disc is loaded, eject it.</p><p></p><p>Obviously the first part of the second clause is the problem. Once again it comes down to the fact that it's difficult/impossible to distinguish between those two scenarios. Quite frankly though, I don't understand why there needs to be any logic there in the first place. The person has selected the "eject" menu item. Therefore shouldn't MP just open the tray? Any objections or flaws in my logic?</p><p></p><p><em>P.S.: the same bug applies to the pictures plugin/section, but the video plugin/section does exactly what I have proposed. Yay for code duplication and people that don't check all the similar code when they fix bugs! <img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" class="smilie smilie--sprite smilie--sprite3" alt=":(" title="Frown :(" loading="lazy" data-shortname=":(" /></em></p><p></p><p></p><p>This is the expected result given the dissection above. However as previously indicated, due to the difficulty in distinguishing closed empty tray from open tray, I think we should modify this so that eject only ejects and load only loads. It would be nice to be smart, but functional is the ultimate priority.</p><p></p><p></p><p>Hmmm, I can't reproduce with 1.7.1.</p><p>Using either the load or eject ( <img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" class="smilie smilie--sprite smilie--sprite11" alt=":rolleyes:" title="Roll Eyes :rolleyes:" loading="lazy" data-shortname=":rolleyes:" /> ) menu items to close the tray containing an audio CD does not give me duplicates. Furthermore, when the tray is closed, my item text changes from "(Z<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" class="smilie smilie--sprite smilie--sprite1" alt=":)" title="Smile :)" loading="lazy" data-shortname=":)" /> CD/DVD" to "(Z<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" class="smilie smilie--sprite smilie--sprite1" alt=":)" title="Smile :)" loading="lazy" data-shortname=":)" /> Audio CD". That's with shares view in the music section.</p><p>Can you reproduce this [USER=97107]@tony72[/USER] ?</p><p></p><p></p><p>You're very welcome, but really it should be me thanking you and CyberSimian for bringing this to our attention and taking the time to test the fix. Again, it's very much appreciated. <img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" class="smilie smilie--sprite smilie--sprite19" alt=":notworthy:" title="Not Worthy :notworthy:" loading="lazy" data-shortname=":notworthy:" /></p><p></p><p></p><p>I think my comments above should cover both of these points. Briefly:</p><ol> <li data-xf-list-type="ol">Your memory is correct. The eject button has never closed the tray, and I don't intend to change that.</li> <li data-xf-list-type="ol">I think we should fix the inconsistency with the context menu by having the "eject" item only eject/open... and that would solve the problem of not being able to open an empty drive using the context menu.</li> </ol><p></p><p>Hopefully I've answered all the questions. All that remains to be mentioned are the attached patches for 1.12, 1.14 and 1.15 PR. Please test as you're able, and be sure to cover the videos, music and pictures sections. As far as I can tell those are all the places that use the same eject/load functions.</p></blockquote><p></p>
[QUOTE="mm1352000, post: 1185040, member: 82144"] Thanks for testing guys, and for your detailed reports - much appreciated. (y) Let me try to cover off all of the points you've raised. Apologies in advance for the length. Please bear with me. That's fine. Yeah, that DLL won't work with 1.12 - sorry. I knew choosing the right version to build would be a bit of a trick. For myself, I still have MP 1.7.1 so I currently can't test anything I build either. I think this may well be a side-effect of the mismatching builds. 1.15 PR would be close enough to 1.14 in most cases, though some things may not work. The addition of Stéphane's changes throws an extra spanner in the works. I'll see if I can build a 1.12 version for your production system, and maybe throw in a 1.15 PR version as optional. I won't be able to include Stéphane's changes - too time consuming. From the code, this is expected: [URL]https://github.com/MediaPortal/MediaPortal-1/blob/master/mediaportal/MediaPortal.Application/MediaPortal.cs#L4170[/URL] Please be aware that it's difficult if not impossible to detect the difference between empty drive and open tray. Thus writing logic to [I]always [/I]open and close the tray appropriately with only one button is tricky... and that's not even considering drives that don't even have a tray. It may be possible to implement some kind of improvement in future but for me and for now I'm strictly limiting scope to fixing bugs without causing any regressions. I confirm that I can reproduce with MP 1.7.1. <sigh> :( The relevant code: [code] case 654: // Eject if (item != null && Util.Utils.getDriveType(item.Path) != 5) { Util.Utils.EjectCDROM(); } else { if (item != null && item.Path != null) { var driveInfo = new DriveInfo(Path.GetPathRoot(item.Path)); if (!driveInfo.IsReady) { Util.Utils.CloseCDROM(Path.GetPathRoot(item.Path)); } else { Util.Utils.EjectCDROM(Path.GetPathRoot(item.Path)); } } } LoadDirectory(string.Empty);[/code] First clause says if the drive is not a CD/DVD/BR drive (removable USB???), just eject [the first/default CD/DVD/BR drive]. To be honest this makes no sense to me, but what the hey. Maybe's it's catch-all code. The second clause is the relevant clause. It says: if a disc is not loaded in the drive (ie. tray is open [B]OR[/B] tray is closed with no disc), close the tray... but if a disc is loaded, eject it. Obviously the first part of the second clause is the problem. Once again it comes down to the fact that it's difficult/impossible to distinguish between those two scenarios. Quite frankly though, I don't understand why there needs to be any logic there in the first place. The person has selected the "eject" menu item. Therefore shouldn't MP just open the tray? Any objections or flaws in my logic? [I]P.S.: the same bug applies to the pictures plugin/section, but the video plugin/section does exactly what I have proposed. Yay for code duplication and people that don't check all the similar code when they fix bugs! :([/I] This is the expected result given the dissection above. However as previously indicated, due to the difficulty in distinguishing closed empty tray from open tray, I think we should modify this so that eject only ejects and load only loads. It would be nice to be smart, but functional is the ultimate priority. Hmmm, I can't reproduce with 1.7.1. Using either the load or eject ( :rolleyes: ) menu items to close the tray containing an audio CD does not give me duplicates. Furthermore, when the tray is closed, my item text changes from "(Z:) CD/DVD" to "(Z:) Audio CD". That's with shares view in the music section. Can you reproduce this [USER=97107]@tony72[/USER] ? You're very welcome, but really it should be me thanking you and CyberSimian for bringing this to our attention and taking the time to test the fix. Again, it's very much appreciated. :notworthy: I think my comments above should cover both of these points. Briefly: [LIST=1] [*]Your memory is correct. The eject button has never closed the tray, and I don't intend to change that. [*]I think we should fix the inconsistency with the context menu by having the "eject" item only eject/open... and that would solve the problem of not being able to open an empty drive using the context menu. [/LIST] Hopefully I've answered all the questions. All that remains to be mentioned are the attached patches for 1.12, 1.14 and 1.15 PR. Please test as you're able, and be sure to cover the videos, music and pictures sections. As far as I can tell those are all the places that use the same eject/load functions. [/QUOTE]
Insert quotes…
Verification
Post reply
Forums
MediaPortal 1
Quality Assurance
Bugreports
Archive
ACTION_EJECTCD doesn't work
Contact us
RSS
Top
Bottom