advplyr / audiobookshelf

Self-hosted audiobook and podcast server
https://audiobookshelf.org
GNU General Public License v3.0
6.54k stars 466 forks source link

[Bug]: Fail to embed meta tags when a description starts with a double quote #1044

Closed andrewgdunn closed 1 year ago

andrewgdunn commented 2 years ago

Describe the issue

The ability to match then M4B encode is incredible.

I'm running into an issue when using the M4B encoder that looks like this:

[2022-10-04 16:05:59] ERROR: [toneHelpers] tagAudioFile: Failed for "/metadata/cache/items/li_9y6kvoa37spanbbweg/A Single Thread (2019).m4b" Error: The path '/(People) "The best-selling novelist has done a masterful job 
of depicting the circumstances of a generation of women we seldom think about: 
the mothers, sisters, wives and fiances of men lost in World War I, whose job it
was to remember those lost but not forgotten." (Associated Press) 1932. After 
the Great War took both her beloved brother and her fiancé, Violet Speedwell has
become a "surplus woman," one of a generation doomed to a life of spinsterhood 
after the war killed so many young men. Yet Violet cannot reconcile herself to a
life spent caring for her grieving, embittered mother. After countless meals of 
boiled eggs and dry toast, she saves enough to move out of her mother's place 
and into the town of Winchester, home to one of England's grandest cathedrals. 
There, Violet is drawn into a society of broderers - women who embroider 
kneelers for the Cathedral, carrying on a centuries-long tradition of bringing 
comfort to worshippers. Violet finds support and community in the group, 
fulfillment in the work they create, and even a growing friendship with the 
vivacious Gilda. But when forces threaten her new independence and another war 
appears on the horizon, Violet must fight to put down roots in a place where 
women aren't expected to grow. Told in Chevalier's glorious prose, A Single 
Thread is a timeless story of friendship, love, and a woman crafting her own 
life.' is too long, or a component of the specified path is too long.
[2022-10-04 16:21:24] ERROR: [toneHelpers] tagAudioFile: Failed for "/metadata/cache/items/li_zdfe9izqh5hxrx95ym/A Spool of Blue Thread (2015).m4b" Error: The path '/This is how Abby Whitshank always begins the story of how she 
fell in love with Red that day in July 1959. The Whitshanks are one of those 
families that radiate togetherness: an indefinable, enviable kind of 
specialness. But they are also like all families, in that the stories they tell 
themselves reveal only part of the picture. Abby and Red and their four grown 
children have accumulated not only tender moments, laughter, and celebrations, 
but also jealousies, disappointments, and carefully guarded secrets. From Red's 
father and mother, newly arrived in Baltimore in the 1920s, to Abby and Red's 
grandchildren carrying the family legacy boisterously into the 21st century, 
here are four generations of Whitshanks, their lives unfolding in and around the
sprawling, lovingly worn Baltimore house that has always been their anchor.  
Brimming with all the insight, humor, and generosity of spirit that are the 
hallmarks of Anne Tyler's work, A Spool of Blue Thread tells a poignant yet 
unsentimental story in praise of family in all its emotional complexity. It is a
novel to cherish.' is too long, or a component of the specified path is too 
long.

I believe this could be manifesting from a couple things:

Steps to reproduce the issue

  1. Have a long description field for a book
  2. Initiate M4B encoding

Audiobookshelf version

v2.2.0

How are you running audiobookshelf?

Docker

andrewgdunn commented 2 years ago

Adding: When using the embed metadata feature the UI reports success but errors in the console (for this same issue).

andrewgdunn commented 2 years ago

Maybe a suggestion is to just ignore the description field when attempting to interact with metadata?

andrewgdunn commented 2 years ago

Just looking for more test cases I think it's clearly an escape character issue:

This is the exact payload for the description field:

"I gulped down Ballistic in one long read, staying awake half the night, and now I want the next one!" (George R. R. Martin) There is a personal price to pay for having aligned with the wrong side in a reckless war. For Aden Jansen it's the need to adopt a new identity while keeping his past hidden. Now he's integrated himself aboard the Zephyr, a merchant ship smuggling critical goods through dangerous space. But danger is imminent on planet Gretia, as well. Under occupation, torn between postwar reformers and loyalists, it's a polestar for civil unrest. Meanwhile an occupation forces officer is pulled right back into the fray when the battle alarm is raised, an ambitious heiress is entangled in a subversive political conspiracy, and an Allied captain is about to meet the enemy head-on. As Aden discovers, the insurgents on Gretia - and in space - are connected, organized, and ready to break into full-scale rebellion. History is threatening to repeat itself. It's time that Aden rediscovers who he is, whom he can trust, and what he must fight for now.

Here is the error:

[2022-10-04 17:02:36] ERROR: [toneHelpers] tagAudioFile: Failed for "/audiobooks/Kloos, Marko/Palladium Wars/Book 2 - Ballistic (2020)/Marko Kloos - The Palladium Wars, Book 2 - Ballistic.m4b" Error: Could not find file '/(George R. R. Martin) There is a personal price to 
pay for having aligned with the wrong side in a reckless war. For Aden Jansen 
it's the need to adopt a new identity while keeping his past hidden. Now he's 
integrated himself aboard the Zephyr, a merchant ship smuggling critical goods 
through dangerous space. But danger is imminent on planet Gretia, as well. Under
occupation, torn between postwar reformers and loyalists, it's a polestar for 
civil unrest. Meanwhile an occupation forces officer is pulled right back into 
the fray when the battle alarm is raised, an ambitious heiress is entangled in a
subversive political conspiracy, and an Allied captain is about to meet the 
enemy head-on. As Aden discovers, the insurgents on Gretia - and in space - are 
connected, organized, and ready to break into full-scale rebellion. History is 
threatening to repeat itself. It's time that Aden rediscovers who he is, whom he
can trust, and what he must fight for now.'.

It seems as if many of audibles descriptions have quotes in them.

advplyr commented 2 years ago

Yeah this is most certainly an issue of escaping the quotes. Thanks for the test data

advplyr commented 2 years ago

I've been testing this and it is specific to the first character being a double quote.

Currently we are using nodes child_process to call the tone CLI. By default the child_process is escaping the arguments automatically which can be disabled but I'm not sure we want to do that. Abs is using the node-tone package I wrote to make calls to tone CLI.

I wasn't able to find a quick fix or workaround unfortunately.

andrewgdunn commented 2 years ago

So, I didn't get to test it, but does M4B have a limit for the comment field? I believe the implementation in Abs right now combines both Comment and Description so you could potentially get a 2x depending on how you scraped metadata.

Would a "fix" for now be to just not attempt to embed the Comment and Description in the M4B?

advplyr commented 2 years ago

I'm not sure about character limits but that would be good to lookup. This issue is definitely the double quote at the beginning and not the character limit. I used Comment and Description so we could use the same object when embedding into mp3 and m4b. That can be updated to use either or but I'm not sure it matters that much.

advplyr commented 1 year ago

Fixed in v2.2.2