Bug 220777 - "Encode file extension" value not passed to path in "Encode File Format"
Summary: "Encode file extension" value not passed to path in "Encode File Format"
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: grip
Version: 5
Hardware: All
OS: Linux
medium
low
Target Milestone: ---
Assignee: Adrian Reber
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2006-12-26 17:12 UTC by George Skuse
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version: grip-3.2.0-15.fc6
Clone Of:
Environment:
Last Closed: 2007-03-18 00:10:52 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description George Skuse 2006-12-26 17:12:24 UTC
Description of problem:
Files encoded to formats other than ogg are still given a file extension of
"ogg".  While the "Encode file extension" is set to something like "mp3", the
"Encode File Format" line does not make use of that setting (and instead
hardcodes "ogg").  

The "Encode File Format" line should be changed for each predefined Encoder
(oggenc, bladenc, lame, flac, etc...)
   from: "~/ogg/%A/%d/%n.ogg"  
   to:   "~/ogg/%A/%d/%n.%x"  
(%x holds the value from the "Encode file extension" field".)

A better fix might be change the default path as well, such that mp3 and flac
files aren't put into the "ogg" directory...

Version-Release number of selected component (if applicable):
grip-3.2.0-12.fc5

How reproducible:
Always

Steps to Reproduce:
1. In Grip:  "Config" tab -> "Encode" subtab -> "Encoder" subsubtab
2. Select: Encoder = oggenc
3. Change "Encode file extension to something other than "ogg"
4. Rip a CDttrack: .  The same holds true if you use a third-party encoder: you
still get oggs.
  
Actual results:
The ripped audio track in ~/ogg/artist/disk/ has an extension of "ogg"

Expected results:
The track should have the extension you entered in the "Encode file extension"
field.

Additional info:
Note that the same holds true if you use a third-party encoder: you still get
files ending in ogg.

Comment 1 Adrian Reber 2007-01-02 12:50:00 UTC
Just to make sure that I understand it correctly... If you put
"~/ogg/%A/%d/%n.%x" it works as expected? So only if you have a hardcoded value
in "Encode file format" it actually uses the hardcoded file format?

Comment 2 George Skuse 2007-01-02 22:19:58 UTC
Yes, if you put "~/ogg/%A/%d/%n.%x" in "Encode file format"  AND "ogg" in
"Encode file extension", then it works as expected.  

For mp3 encoders, the  "Encode file extension" should be set to "mp3". 
Likewise, for flac it should be set to "flac".


Comment 3 Adrian Reber 2007-01-02 23:31:52 UTC
So this is not really a bug but wrong default settings.

Looking at the source, however, it seems that for most encoders it is set to the
correct extension (although the extension is always hardcoded and the value from
the file extension entry is not used).

I just looked again at the code and I think I now really understood your problem
and I also see it.

The value of "Encode file extension", however, seems to be always correct
(looking at the source):

It is ogg for oggenc, flac for flac and mp3 for the mp3 encoders.

I will release an update for FC5, FC6 and the devel branch.

Please reopen the bug if the new release does not fix this for you and please be
aware the before reopening the bug to remove all config files (.grip*) just in
case some information is stored in the config files.

Comment 4 George Skuse 2007-01-06 22:35:43 UTC
Thanks for the fix. Things look better, but could still use a little polish.
FYI: I removed ~/.grip* and then updated to grip.i386 1:3.2.0-15.fc5

Some issues:

1. File paths should be made more generic...
oggenc is placing oggs in a path that includes says "mp3":
   ~/mp3/%A/%d/%n.%x

flac is doing the same thing.

Proposed fix: put all ripped files, regardless of format, in single directory. 
Whereas banshee creates "~/Music/" when started, I suggest that grip be
configured to match.  Thus, the "Encode file format" field for each encoder
would be changed to "~/Music/%A/%d/%n.%x"  (Config -> Encode -> Encoder).


2. m3u file location needs to be updated to match...
Config -> Encode -> Options
"M3U File Format" is currently:  "~/ogg/%A-%d.m3u"
It should be changed to match above: "~/Music/%A-%d.m3u" 


3. Rip (wav) file location should be updated as well...
Config -> Rip -> Ripper
"Rip file format" is currently: "~/ogg/%A/%d/%n.wav"
Could we change it to match above as well?  (ie: "~/Music/%A/%d/%n.wav")


4.  bladeenc's path is set incorrectly (to "/usr/bin/oggenc").  This should be
changed to "/usr/bin/blandeenc" (or else, maybe just bladeenc).


Again, thanks for taking the time to look at this :)


P.S. : I'm not sure if the "Music" folder is the the _best_ way to go... I'm
merely offering it as an alternative to having separate ogg, flac and mp3
directories or to putting oggs and flacs in the mp3 directory.

Comment 5 Moritz Barsnick 2007-01-10 14:59:10 UTC
I agree with comment #4. I was looking forward to the new defaults, also moved 
my ~/.grip directory, and was disappointed about the results.

Even if I ignore that grip places everything in ~/ogg/ (that may just be the 
author's preference), it's not really useful switching to lame as encoder and 
getting:
mp3fileformat ~/ogg/%A/%d/%n.ogg
mp3extension mp3

I can't try right now (I'm working remotely and there's no CD in the drive), 
but I guess lame/grip then names its encoded MP3 files "*.ogg". So: same old 
bug still there, still missing %x! ;-)

Moritz

Comment 6 George Skuse 2007-01-10 16:10:02 UTC
Further testing shows that the "Encode file format" field is grip-wide and not
encoder-specific.  Grip wants to put all files, from all encoders, in the same
directory.  I think this supports having the directory name be more generic.  

If it's really necessary to put oggs in a seperate directory from mp3s, flacs,
etc, the FileFormat field could be changed to: "~/%x/%A/%d/%n.%x  This would put
mp3s in:
   ~/mp3/%A/%d/%n.mp3
and oggs in:
   ~/ogg/%A/%d/%n.ogg

I still prefer using "~/Music/%A/%d/%n.%x" as I don't think the 'average user'
cares about the format - they care about getting to their "Music".


re: comment 5 - I didn't find that the file format lines were missing %x.  Try
deleting all .grip* files in ~/ (there should be a number of them) and report
back.  The files that I see as needing to be deleted are listed below.

~/.grip
~/.grip-bladeenc
~/.grip-flac
~/.grip-gogo
~/.grip-l3enc
~/.grip-lame
~/.grip-oggenc
~/.grip-other
~/.grip-xingmp3enc

--George

Comment 7 Adrian Reber 2007-01-10 16:41:45 UTC
I will not do any changes concerning the direrctory names. This is something you
should take upstream. Changing it from one to another makes not much sense to
me. "Music" would of course be more generic than "ogg" or "mp3", but I do not
think this is something which should be changed by the packager (me). Everybody
can change it to would he likes.

That i does have a wrong path for bladeenc this is probably because bladeenc is
not found in your path when you first selected bladeenc and it just took the
value previously defined. I would say this is a bug.

Both problems are from my point of view better handled upstream because this are
general (not packaging related) bugs.

I will close the bug with CURRENTRELEASE because the initial problem is fixed
and released if you want and bigger changes please talk to upstream because I
cannot fix it.

Comment 8 George Skuse 2007-01-10 18:26:56 UTC
I'd say that as the packager, you're tasked with trying to integrate the
upstream with the distribution.  At the very least, we can't leave the "Encode
File Format" set to "~/mp3/%A/%d/%n.%x" as Fedora Core/Extras doesn't include an
mp3 encoder.  If we can't move to ~/Music, I suggest either:

1. changing back to  ~/ogg/%A/%d/%n.%x  (so that it's sane for the encoders that
do ship with Fedora)
  - OR -
2. Changing it to ~/%x/%A/%d/%n.%x (so that the path matches the filetype - oggs
get put in ~/ogg/ and mp3s get put in ~/mp3/ )

Personally, I like 2.

Reopening as I don't think we should introduce a path that includes "mp3" into a
distribution that doesn't include an mp3 encoder.

In case I haven't said it earlier: Thank you for all the time you've spent on this.

--George

Comment 9 Adrian Reber 2007-01-10 19:03:11 UTC
Ah, I see what I did wrong. I have removed a few lines too much from the patch.
The mp3 is of course not correct. Did not see this. Thanks for the hint. I
almost was changing it already to suggestion 2 but I am not sure if I want to
introduce that kind of change because it might the life harder for the users to
find the files. So I would rather go with number 1.
If going with case number two... Where would you put the wav before being
ripped? Also in ~/%x/ ?

Comment 10 George Skuse 2007-01-10 19:39:10 UTC
"If going with case number two... Where would you put the wav before being
ripped? Also in ~/%x/ ?"

Good question... I think %x would evaluate to the _final_ extension of the file
(ogg, mp3, flac, etc...).   If it does, then using "~/%x/%A/%d/%n.wav"  makes
sense.  I wouldn't want it to create ~/wav/ though.



Comment 11 Adrian Reber 2007-03-18 00:10:52 UTC
I have released a new version for the FC-5, FC-6 and devel branch. I hope it
should be correct now.


Note You need to log in before you can comment on or make changes to this bug.