Bug 426558 - abcde aac/m3a tagging post encoding (With Patch)
abcde aac/m3a tagging post encoding (With Patch)
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: abcde (Show other bugs)
7
All Linux
low Severity low
: ---
: ---
Assigned To: Ville Skyttä
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-12-21 21:38 EST by Bill Adams
Modified: 2008-01-23 11:33 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-23 11:33:46 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Patch to use AtomicParsley to Post-Tag Tracks (2.42 KB, patch)
2007-12-21 21:38 EST, Bill Adams
no flags Details | Diff
Patch to the abcde Spec File (1.17 KB, patch)
2007-12-21 21:39 EST, Bill Adams
no flags Details | Diff
New Patch for Post-Tagging M4A Files (2.48 KB, patch)
2007-12-23 18:50 EST, Bill Adams
no flags Details | Diff

  None (edit)
Description Bill Adams 2007-12-21 21:38:04 EST
Description of problem:

abcde is not capable of post-tagging aac/m3a tracks. Attached is a patch to use
AtomicParsley to post-tag m4a tracks. I suppose an AtomicParsley RPM will need
to be generated to -- although the patch checks for AtomicParsley and falls back
to the previous method when it is not available.


Version-Release number of selected component (if applicable):

abcde-2.3.99.6-3.fc7


How reproducible:

always


Steps to Reproduce:
1. abcde -o m3a
  
Actual results:
No tags.

Expected results:
Tags.


Additional info:

Please apply to FC7 and FC8.
Comment 1 Bill Adams 2007-12-21 21:38:04 EST
Created attachment 290282 [details]
Patch to use AtomicParsley to Post-Tag Tracks
Comment 2 Bill Adams 2007-12-21 21:39:24 EST
Created attachment 290283 [details]
Patch to the abcde Spec File

Bump the version, apply the patch, added a changelog entry.
Comment 3 Ville Skyttä 2007-12-22 14:33:44 EST
Have you tried sending this patch upstream (jesus.climent@hispalinux.es)?

It seems that abcde already contains aac/m4a tagging code using faac, it's just
disabled currently.  I'd feel much more comfortable with enabling that than
adding a patch that upstream really should look into first.  More info:
https://bugs.launchpad.net/ubuntu/+source/abcde/+bug/138367
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=449045

Could you test if that patch works for you?

A couple of quick comments about your patch:

+			#  case core dumps with AtomicParsely 0.9.0

s/Parsely/Parsley/

+		echo "WARING: $METAAAC Not Found Not Post-Tagging!"

s/WARING/WARNING
s/Not Found Not Post-Tagging/not found, not post-tagging/

The METAAAC variable name sounds odd to me, I think a better choice would be
ATOMICPARSLEY.
Comment 4 Bill Adams 2007-12-23 18:50:53 EST
Created attachment 290313 [details]
New Patch for Post-Tagging M4A Files

Good day.

o Fixed the spelling errors.

o Changed METAAAC (I was going off off METAFLAC) to ATOMICPARSLEY.

I added post-encoding tagging because of the text in the code "Quick hack
to...." I figured that post-tagging was preferred. If AtomicParsley is not
present, it falls back to inline tagging.

I'll send the patch to the supplied email address and see if it gets accepted.

Thanks for your feedback.
Comment 5 Bill Adams 2008-01-12 17:17:55 EST
FYI: I sent an email on Dec. 23 to jesus.climent at hispalinux.es and have still
heard no response. I just sent another email trying to get a response to if he
will apply the patch to the main branch or not.

Comment 6 Ville Skyttä 2008-01-20 16:24:14 EST
More feedback:

- If AtomicParsley is not available, an ugly error message is printed; I think
that's undesirable: which: no AtomicParsley in ($PATH goes here...)

- If it's not found, another "WARNING: AtomicParsley Not Found Not
Post-Tagging!" message is printed.  Why is this worth a warning in the first
place?  AFAICT faac does the job just fine.  If you want to keep it, I think
some rephrasing would be in order, eg. "WARNING: AtomicParsley not found,
falling back to inline tagging."

- After applying your patch to my work in progress package by hand, if
AtomicParsley was not available, abcde did not fall back to inline faac tagging
(it did not tag at all), and failed to create the real target "Author-Albumname"
directory, leaving just the abcde.xxxxxxxx dir and all the cruft in it behind.

- The core dump commentary in the patch does not give me a warm fuzzy feeling
about AtomicParsley.

Having played around with m4a tagging for a while now and seeing that faac is
the only supported m4a encoder in abcde, I must say that I fail to see the
reason to introduce AtomicParsley support.  faac appears to do the job just
fine, whereas AtomicParsley is another dependency, adds more code, and the
implementation seems slightly broken at the moment.

If you wish to continue improving the patch, please rebase it with abcde
2.3.99.6-5 which will soon hit F8 testing updates and the devel repository.  In
the meantime, you can grab it from CVS or
http://koji.fedoraproject.org/koji/taskinfo?taskID=360697
Comment 7 Bill Adams 2008-01-22 15:18:54 EST
Thanks for the feedback. All good points.

Let's drop this patch/bug in favor of enabling/using faac tagging.

Thanks!
Comment 8 Ville Skyttä 2008-01-23 11:33:46 EST
Ok, let's do that, thanks for the quick reply.

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